brouberol/bo

My personal prose editor with vim-like navigation.

bo's Issues

Re-draw editor when the size of the terminal changes

When the terminal is resized, the app should handle a SIGWINCH signal to resize the editor according to the new terminal size.

Improve type system around Position/Coordinates

Coordinates have been tricky to get right for multiple reasons:

• termion assumes 1-based coordinates, because this is what ANSI assumes (see https://docs.rs/termion/1.5.6/termion/cursor/struct.Goto.html), whereas Editor.cursor_position assumes 0-based coordinates.
• I shoved a x_offset into the Position struct to account for the :ln induced x-shift, but Editor.offset (the viewport x/y offsets) has to use that x_offset attribute and always set it to 0 because it does not need it, as it's an attribute of type Position.

All of that is an open door to offset-related bugs, and has been pretty clunky to use. It just does not fit really well. I think we should redesign the type system into something like:

• AnsiPosition{x, y}: represents the coordinates on screen, and conforms with ANSI by having its origin be (1, 1).
• Position{x, y}: represents the current row/column as indices , ie:current_row = editor.document.rows[x] and current_grapheme = editor.documents.rows[x][y] (pseudocode). The goal is to never having to perform a (0,0)-based to (1,1)-based conversion (or conversely) in editor.rs.
• ViewportOffset{x, y}: number of rows/columns the viewport has been shifted by (from a (0, 0) origin) to scope it to the current view.
• Position.x_offset should be dropped, and an Editor.row_prefix_length u8 attribute should be introduced.

Let's play around with this, to see whether it makes the code easier to deal with, with less arithmetic and traps.

`bo <path>` should open an unsaved `<path>` file instead of the welcome page

Add a `:wq` command to save and quit

Add marker in status bar showing if the file has unsaved changes

Undo fails on line deletion

How to reproduce

plop
lala

Go to line 2
type d
type u

The fact that the document has 1 line seems to clash with "re-growing" it back to 2. Probably because we send the line lengths to Operation.reversed().

Editing a long string at the ends inserts the character in the middle

Reset the history when opening a new document

Restore unsaved changes from swap file

It a swap file is found when opening a file, suggest opening it instead of the actual file, to restore our unsaved changes.

Don't display `+` in the status bar if the file was modified then changed back to its initial state

Our algorithm to detect change is too naive: if the document is opened, a character is inserted and then deleted, the document will still be considered as "dirty". We need to save the document hash when we save it, and compare this value to the current document hash.

Editor/Document.filename should be a `Path`, not a `String`

This will massively simplify the swap filename construction, as String isn't the way to represent a path in Rust (see https://fasterthanli.me/articles/i-want-off-mr-golangs-wild-ride), Path is.

Implement the `J` command

This command would join the current line with the next one.

Support undo/redo for the last edit action

Deleting the last char on a non-empty line deletes the newline of the previous line

Hello
w<Backspace>

causes this to appear

Hellow

instead of

Hello

`d` isn't offset aware

Deleting line 25 with a y offset of 50 deletes line 25, instead of line 75.

Publish to crates.io when cutting a release

Support line wrapping

By using a command, such as :wrap or :linewrap, we'd split lines exceeding the terminal width over multiple terminal lines.

This will probably be complex, as it'll have an impact on line numbering, the go_to_x_y mechanism, etc, so we'd need to find a lightweight solution to compute a document coordinate / terminal coordinate mapping, instead of re-computing it everytime, which would make every coordinate jump O(n).

`A` moves cursor to the penultimate column for offseted lines

That screenshot crops the end of a very lone (offseted) line, after we've pressed A:

Writing 100 characters in a document without any filename causes `bo` to crash.

The Document::swap_filename method assumes that the document has a filename. However, running bo without any argument will open a document without a filename. That method should return an Option<Pathbuf> instead, and handle that case.

Add a (dev only?) command to dump the internal state to a file

I tried using bo as my git editor, and opening a PR caused a weird behavior. I'd like to be able to issue a :dbg command that would dump entire editor state on disk so I could better track down the root cause.

RUSTSEC-2023-0018: Race Condition Enabling Link Following and Time-of-check Time-of-use (TOCTOU)

Race Condition Enabling Link Following and Time-of-check Time-of-use (TOCTOU)

Details
Package	remove_dir_all
Version	0.5.3
URL	XAMPPRocky/remove_dir_all@7247a8b
Date	2023-02-24
Patched versions	>=0.8.0

The remove_dir_all crate is a Rust library that offers additional features over the Rust
standard library fs::remove_dir_all function.

It was possible to trick a privileged process doing a recursive delete in an
attacker controlled directory into deleting privileged files, on all operating systems.

For instance, consider deleting a tree called 'etc' in a parent directory
called 'p'. Between calling remove_dir_all("a") and remove_dir_all("a")
actually starting its work, the attacker can move 'p' to 'p-prime', and
replace 'p' with a symlink to '/'. Then the privileged process deletes 'p/etc'
which is actually /etc, and now your system is broken. There are some
mitigations for this exact scenario, such as CWD relative file lookup, but
they are not guaranteed - any code using absolute paths will not have that
protection in place.

The same attack could be performed at any point in the directory tree being
deleted: if 'a' contains a child directory called 'etc', attacking the
deletion by replacing 'a' with a link is possible.

The new code in this release mitigates the attack within the directory tree
being deleted by using file-handle relative operations: to open 'a/etc', the
path 'etc' relative to 'a' is opened, where 'a' is represented by a file
descriptor (Unix) or handle (Windows). With the exception of the entry points
into the directory deletion logic, this is robust against manipulation of the
directory hierarchy, and remove_dir_all will only delete files and directories
contained in the tree it is deleting.

The entry path however is a challenge - as described above, there are some
potential mitigations, but since using them must be done by the calling code,
it is hard to be confident about the security properties of the path based
interface.

The new extension trait RemoveDir provides an interface where it is much
harder to get it wrong.

somedir.remove_dir_contents("name-of-child").

Callers can then make their own security evaluation about how to securely get
a directory handle. That is still not particularly obvious, and we're going to
follow up with a helper of some sort (probably in the fs_at crate). Once
that is available, the path based entry points will get deprecated.

In the interim, processes that might run with elevated privileges should
figure out how to securely identify the directory they are going to delete, to
avoid the initial race. Pragmatically, other processes should be fine with the
path based entry points : this is the same interface std::fs::remove_dir_all
offers, and an unprivileged process running in an attacker controlled
directory can't do anything that the attacker can't already do.

See advisory page for additional details.

Trailing spaces should be highlighted in red

Remove trailing spaces when saving a file

Lines exceeding terminal width get cut after `term_width` chars when hitting Enter

Here I have entered more characters than the terminal width.

I then hit Enter and the line seems to be cut at term_width chars.

which is confirmed after I tweaked the status bar to display the terminal size.

RUSTSEC-2021-0139: ansi_term is Unmaintained

ansi_term is Unmaintained

Details
Status	unmaintained
Package	ansi_term
Version	0.12.1
URL	ogham/rust-ansi-term#72
Date	2021-08-18

The maintainer has adviced this crate is deprecated and will not
receive any maintenance.

The crate does not seem to have much dependencies and may or may not be ok to use as-is.

Last release seems to have been three years ago.

Possible Alternative(s)

The below list has not been vetted in any way and may or may not contain alternatives;

• anstyle
• nu-ansi-term
• yansi

See advisory page for additional details.

Build crate documentation and host it online

Record a terminal session and display it at the top of the README

This might help with giving potential users/contributors decide whether it's worth their investment, and it'd be a nice addition to the README.

I might have to video-edit the recording to display pressed keys. We'll see.

Support block indent/unindent with `>` and `<`

Probably depends on #17 for the command structure.

Support edit actions specifiers

Commands such as dw, cb, dd, etc, have a prefix and a specifier. For example dw has the d (delete) prefix, and the w (word) specifier, meaning that we'd want to delete the next word. We could also have d} to delete the next paragraph, etc.

We should support such commands, that will get especially for yank/paste.

Long lines + line numbers messes up the output

Before (without :ln enabled)

I then enter :ln and the output is completely mixed up:

Another hit on : further messes up the output 😱, lines are all mixed up..

Add a command to force quit if the file has unsaved changes

Support yanking/pasting

Probably depends on #17 to support the command structure.

Don't display the program name in an non-empty buffer

As soon as we start typing anything in an unnamed (no filename) editor, the mention bo <version> should disappear.

Maintain a history per document instead of a global editor attribute

Automatically publish the release changelog when publishing a new release

As per https://github.com/taiki-e/parse-changelog I need to start maintaining a changelog parse-changelog can, well, parse.

Prevent escaping the document bounds by clicking

When processing a Mouse click event, we shouldn't blindly go to the provided coordinates, as the program will then crash at the next motion, as the cursor isn't in an existing Row.

Add a `r<char>` command replacing the character under the cursor

Move cursor to the top left when displaying help

Restore the cursor when going back to the main screen.

Replace tabs by 4 spaces at insert time

Support `~` when opening a file

Long lines bleed through the line number prefix space

Automatically dump changes to a swap file every `n` unsaved changes

Every, say, 100 keystrokes, the file should be dumped to a hidden swap file, as long as the current file is unsaved. Delete the swap file when saving the file.

Add a command to rename/save as

Potential commands:

• :saveas <filename>
• :w <filename>

I think I prefer the second one, as it's what vim implements, and it's also more intuitive.

Handle SIGTSTP to support sending ot background via Ctrl-z

Run clippy as part of the CI

The CI should run cargo clippy --all-targets --all-features -- -D warnings

Terminal paste (`Ctrl + v`) "eats up" half of the characters.

Ctrl-v is a standard to paste the content of the clipboard, which sends the stream to the process' stdin. However pasting into bo` seems to only display .. half of these characters.

Demo

Pasting the word "Char" in bo, in INSERT mode

Investigation

I was able to track down the issue to

bo/src/terminal.rs

Line 43 in 3ac0f8e

let opt_key = io::stdin().lock().events().next();

. I added a utils:log call to print out the content of eachopt_key variable, wrapping the event, and I could only see half of the events being received:

$ tail -f bo.log
...
Some(Ok(Key(Char('i'))))  # switch to insert mode
Some(Ok(Key(Char('C'))))
Some(Ok(Key(Char('a'))))
...

So this looks like an issue with how we receive stdin events.

Support replacement of a search pattern

A possible syntax could be

/search_pattern

for simple search, and

/search_pattern/replace_by

We should probably handle / being part of the search pattern by escaping it: \/.

Allow multiple search ocurrences to be found in the same line