[Toybox] [PATCH] vi: add "w <filename>".

Jarno Mäkipää jmakip87 at gmail.com
Mon Jan 30 02:01:07 PST 2023


On Sun, Jan 29, 2023 at 9:04 PM enh via Toybox <toybox at lists.landley.net> wrote:
>
> (Issues reported by an Android partner; basically "vi" with no arguments
> is broken, and "vi foo" where foo doesn't yet exist is broken, and even
> if they weren't, ":w filename" isn't implemented. This patch fixes the
> last of those more convincingly than the other two.)

To me it looks like there has been some regression since my last
commit as on my local working copy creating empty file works.
commit I had on my old working directory:
6f0f61ad4430f3df72b118371a8b1643ddb69429

In my system current vi from master does not even compile. So I cant
briefly check what is current state of this toys/pending/vi.c
In function 'open',
    inlined from 'write_file' at toys/pending/vi.c:575:13:
/usr/include/x86_64-linux-gnu/bits/fcntl2.h:50:11: error: call to
'__open_missing_mode' declared with attribute error: open with O_CREAT
or O_TMPFILE in second argument needs 3 arguments

>
> Don't crash (or write '(null).swp') when started with no filename;
> refuse to write without a filename (with ":w filename" being the
> workaround).

>
> Also fix insert to work when started with a filename where no such file
> yet exists. (This appears to still be buggy in some cases, depending
> on the exact sequence of editing operations performed. An 'i' followed
> by an 'A' for example suggests that there's an off-by-one in the whole
> block_list/slice_list thing. I still think we should just replace that
> with an easy to get right https://en.wikipedia.org/wiki/Gap_buffer
> instead, or even just the trivial case where the gap is always at the
> end! Especially when we come to dealing with stuff like undo/redo...)

I tried to implement buffer handling into few interface functions
(ones starting with "text_" such as text_sol, text_eol....) Therefore
changing to different buffer mechanism at this point should be
trivial. If someone with spare time decides to implement Gap buffer,
linked list, line span, rope etc.. they should look into those
functions. But same bugs will mostly still exist since I have feeling
that text_ functions work as intended but vi command implementations
are using them wrong.

These errors not relating to buffer mechanism itself but "user
interfacing". Way the "i" and "A" etc are just probably implemented is
wrong and do not handle all the corner cases. In case for commands
like 'A', 'dd', 'yy' that deal with line endings, when implementing
you need to think do you need to go before, at or after newline
character. Version that still compiles to me
(6f0f61ad4430f3df72b118371a8b1643ddb69429) works in my perspective but
if there is something miss behaving its best to write unit test into
tests/vi.test by using -s argument to run sequence of operations for
input file.

I think better would be first fix user command handling to actually
handle all inputs correctly, but that takes dedication since vi and ex
command syntax is very complex. And perhaps after that implementing
rest of useful commands. Also drawing actual text buffer to output is
currently just mostly debug implementation :)

But I am not totally against of Gap buffer, its good for small files.
But its not silver bullet for implementing vi since its not text
editor where user is constantly in insert mode and moving with arrow
keys.

>
> Also add basic error reporting similar to hexedit. This (and the notion
> of a status line that would help make both even better, in particular
> removing the bogus "should be long enough" sleeps, should probably end
> up in tty.c, but this is better than nothing.)
>
> Also simplify the three (0/1/-1) return values from run_ex_cmd(),
> which appears to have been a bug anyway.
>
> Also document that TT.modified isn't actually updated (a bug that would
> be a lot easier to fix with a simpler text representation). This means
> that although I've added the missing ":q" warning for modified files, it
> doesn't actually ever fire yet.
>
> Bug: https://issuetracker.google.com/242636400
> ---
>  toys/pending/vi.c | 114 +++++++++++++++++++++++++++++-----------------
>  1 file changed, 71 insertions(+), 43 deletions(-)
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net

Anyway its nice that someone has interest on using this very early
prototype version of text editor. I dont mind if someone decides to do
even some drastic code changes to make it more usable and robust.

br.
Jarno


More information about the Toybox mailing list