------- Original Message -------
On Sunday, October 1st, 2023 at 1:33 PM, Jarno Mäkipää <jmaki...@gmail.com>
wrote:
> On Sun, Oct 1, 2023 at 6:30 PM Oliver Webb aquahobby...@proton.me wrote:
>
> > ------- Original Message -------
> > On Sunday, October 1st, 2023 at 7:50 AM, Jarno Mäkipää jmaki...@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.
I was unaware of this. I haven't read through the entirety of vi.c to see
exactly what it does.
But if reloading the buffer and mapping files is already built it then that
would make r[ead],
e[dit] and the !system commands a lot easier to implement.
> > > 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 aquahobby...@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.
The below patch replaces tabs with 2 spaces in vi.c.
> http://landley.net/toybox/design.html#codestyle
>
> -Jarno
- Oliver Webb <aquahobby...@proton.me>
From 44b57a48a109375bc11be007ba6c0c3f7dcc7f5b Mon Sep 17 00:00:00 2001
From: Oliver Webb <aquahobby...@proton.me>
Date: Sun, 1 Oct 2023 13:40:12 -0500
Subject: [PATCH] Replace tabs with spaces in vi.c
---
toys/pending/vi.c | 86 +++++++++++++++++++++++------------------------
1 file changed, 43 insertions(+), 43 deletions(-)
diff --git a/toys/pending/vi.c b/toys/pending/vi.c
index 385ad7a8..6b37f1b4 100644
--- a/toys/pending/vi.c
+++ b/toys/pending/vi.c
@@ -1313,20 +1313,20 @@ static void draw_page();
static int get_endline(void)
{
- int cln, rln;
- draw_page();
- cln = TT.cur_row+1;
- run_vi_cmd("G");
- draw_page();
- rln = TT.cur_row+1;
- run_vi_cmd(xmprintf("%dG", cln));
- return rln+1;
+ int cln, rln;
+ draw_page();
+ cln = TT.cur_row+1;
+ run_vi_cmd("G");
+ draw_page();
+ rln = TT.cur_row+1;
+ run_vi_cmd(xmprintf("%dG", cln));
+ return rln+1;
}
// Return non-zero to exit.
static int run_ex_cmd(char *cmd)
{
- int startline = 1, ofst = 0, endline;
+ int startline = 1, ofst = 0, endline;
if (cmd[0] == '/') search_str(cmd+1);
else if (cmd[0] == '?') {
@@ -1350,22 +1350,22 @@ static int run_ex_cmd(char *cmd)
TT.vi_mov_flag |= 0x30000000;
}
- else if (*(cmd+1) == 'd') {
- run_vi_cmd("dd");
- run_vi_cmd("k");
- }
-
- // Line Ranges
- else if (*(cmd+1) >= '0' && *(cmd+1) <= '9') {
- if (strstr(cmd, ",")) {
- char *tcmd = xmalloc(strlen(cmd));
- sscanf(cmd, ":%d,%d%[^\n]", &startline, &endline, tcmd+2);
- cmd = tcmd;
- ofst = 1;
- } else run_vi_cmd(xmprintf("%dG", atoi(cmd+1)));
- } else if (*(cmd+1) == '$') {
- run_vi_cmd("G");
- } else if (*(cmd+1) == '%') {
+ else if (*(cmd+1) == 'd') {
+ run_vi_cmd("dd");
+ run_vi_cmd("k");
+ }
+
+ // Line Ranges
+ else if (*(cmd+1) >= '0' && *(cmd+1) <= '9') {
+ if (strstr(cmd, ",")) {
+ char *tcmd = xmalloc(strlen(cmd));
+ sscanf(cmd, ":%d,%d%[^\n]", &startline, &endline, tcmd+2);
+ cmd = tcmd;
+ ofst = 1;
+ } else run_vi_cmd(xmprintf("%dG", atoi(cmd+1)));
+ } else if (*(cmd+1) == '$') {
+ run_vi_cmd("G");
+ } else if (*(cmd+1) == '%') {
startline = 1;
endline = get_endline();
ofst = 1;
@@ -1373,21 +1373,21 @@ static int run_ex_cmd(char *cmd)
else show_error("unknown command '%s'",cmd+1);
- if (ofst) {
- draw_page();
- int cline = TT.cur_row+1;
- *(cmd+ofst) = ':';
- run_vi_cmd(xmprintf("%dG", startline));
- for (; startline <= endline; startline++) {
- run_ex_cmd(cmd+ofst);
- run_vi_cmd("j");
- }
- run_vi_cmd(xmprintf("%dG", cline));
- // Screen Reset
- ctrl_f();
- draw_page();
- ctrl_b();
- }
+ if (ofst) {
+ draw_page();
+ int cline = TT.cur_row+1;
+ *(cmd+ofst) = ':';
+ run_vi_cmd(xmprintf("%dG", startline));
+ for (; startline <= endline; startline++) {
+ run_ex_cmd(cmd+ofst);
+ run_vi_cmd("j");
+ }
+ run_vi_cmd(xmprintf("%dG", cline));
+ // Screen Reset
+ ctrl_f();
+ draw_page();
+ ctrl_b();
+ }
}
return 0;
}
@@ -1678,9 +1678,9 @@ void vi_main(void)
case 'i':
TT.vi_mode = 2;
break;
- case CTL('D'):
- ctrl_d();
- break;
+ case CTL('D'):
+ ctrl_d();
+ break;
case CTL('B'):
ctrl_b();
break;
--
2.34.1
_______________________________________________
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
