[Toybox] [PATCH] vi.c: line ranges, fixed line gotos, CTRL-D, etc

[Jarno Mäkipää]

On Sun, Oct 1, 2023 at 6:30 PM Oliver Webb <aquahobbyist at proton.me> wrote:
>
> ------- Original Message -------
> On Sunday, October 1st, 2023 at 7:50 AM, Jarno Mäkipää <jmakip87 at gmail.com> wrote:
>
>
> > Hello Oliver
> >
> > ex mode command parsing is not something that has been designed
> > carefully. Its more of hack to accept write file and exit commands in
> > order to make editor somewhat usable. Even the vi mode command
> > execution is not very good. At least one issue is that it has function
> > dispatch table that handles most of the commands and also switch case
> > that handles some... It should probably have only one or the other and
> > not both...
> >
> > I see that you added some direct calls to run_vi_cmd() in ex mode and
> > other places. I am not sure is that going to work out since run_vi_cmd
> > was supposed to be only called when in vi mode.
>
> Originally, I wanted to stay away from run_vi_cmd() for speed reasons. And
> instead call what run_vi_cmd called. As a example: after some debug
> printf-ing, I figured out what the "G" vi command did (calling vi_go())
> and, I called vi_go(NUMBER, 1, 0) instead of run_vi_cmd(xmprintf("%dG",NUMBER)),
> Problems arose with this though that don't normally appear whilst
> doing run_vi_cmd("G"). Not moving around at the top or bottom files, Weird
> stuff happening when moving around with "hjkl" before calling the command,
> etc. It must be some setup procedure that run_vi_cmd does before or after
> calling the vi_go() in the dispatch table because I call it in the
> exact same manner with the same arguments. I haven't done enough debugging to
> pin down the reason, and since run_vi_cmd() works I just called that.
>
> > I don’t really use ex
> > commands other than r, w, q so I don’t really have idea how they work.
> > But if you wish to implement some commonly used ones,
>
> I am planning to implement 'g' ('v' follows once you have that)
> and maybe 's' if I can manage it reasonably.
> After some consideration 'p' is obsolete in the same way that
> 'i' and 'a' (the ex commands, not the vi ones) are.
> It may be useful for g/re/p-ing though a file for lines far apart.
> But after looking at how error messages work in vi, I honestly
> think it would be more trouble then it's worth. 'e' would be nice.
> But reloading the buffer and essentially resetting the editor is also probably
> also more trouble then it's worth. I plan to implement system commands to give
> it the UNIX modularity that's ever so useful. and if I am going to be reading
> from the stdout of a command and placing it back into a file, reading a file with 'r'
> wouldn't be that heavy of a lift.

e[dit] and r[ead] are trivial to implement this point. all the
plumbing for loading and reloading buffer is already there and also
mapping another file into current position of buffer.

>
> > you could try to
> > implement better parsing logic for ex commands so that they are parsed
> > similar logic as vi mode: do movement, execute action based on
> > movement, repeat if counter has been given.
>
> A lot of ex commands don't necessarily do movement, 's' for example for example shouldn't
> effect cursor position. If a "counter" (Line range) is given, it loops through the line range
> and executes the command for each line before going down to the next one, this is why 'd' goes up after
> deleting the line, It should stay on the current line so we don't skip any.

By looking man page for ex. There is this [2addr]
cmd[buffer][count][flags] logic that some commands follow, where 2addr
argument is relatively same thing as movement in vi. By meaning that
you search beginning and end positions for operation. But then again
most of commands dont exactly follow this and there is special case
after special case. making generic ex command parsing logic might be
pain to implement.

>
> > If you plan to add features you could add some tests into /tests/vi.test
>
> I have added a few test cases for the ex commands in the patch attached to this email
>
> > While tests don’t cover how buffer looks visually in terminal
> > emulator, it can cover how buffer is edited and stored on output file.
> > This can detect at least some regressions.
> >
> > -Jarno
>
> - Oliver Webb <aquahobbyist at proton.me>

I was looking at the implementation you have added and noticed there
is some code indentation issues, could you replace tabs with 2 spaces
in order to follow project rules. Aim is to follow this style guide.

http://landley.net/toybox/design.html#codestyle

-Jarno