[Toybox] Errors While Compling fold and/or defconfig: xgetdelim not found

2023-10-01 Thread Oliver Webb via Toybox
When I saw that fold got promoted, I pulled the repo and ran make.  Instead of
compiling cleanly, fold.c gave a error that a 'xgetdelim()' didn't exist.
"grep -R"-ing through the code, it was only mentioned in fold.c.

I tried to fix this originally by trying to implement xgetdelim, but
the problem is probably that some un-commited changes were made to
lib/xwrap.c. which fold.c based itself on.  then when _only_ fold.c got
committed were errors apparent.

I can't get a good fix for this command that looks reasonable, compiles,
and passes it's tests; Best I have done was make something that passed the
first test case.

Rob, If you have some changes you made to xwrap.c. Can you commit them?
"make defconfig toybox" no longer works. And everything about this error makes
me think this was a "Develop and test fold.c under a uncommited version of
xwrap.c with xgetdelim(), then only update fold.c" issue. Especially since
versions of fold before it's promotion don't use xgetdelim and compile ceanly

- Oliver Webb 

___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net


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

2023-10-01 Thread Rob Landley
On 10/1/23 13:33, Jarno Mäkipää wrote:
>> 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.

In the absence of a test suite, could we at least fill out the help text to list
the implemented features? (You don't even have to describe what they do, just...
what's there?)

I can do it myself eventually, but I'm already spinning enough plates that
swap-thrashing overhead is dominating the runtime...

Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net


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

2023-10-01 Thread Oliver Webb via Toybox

--- Original Message ---
On Sunday, October 1st, 2023 at 1:33 PM, Jarno Mäkipää  
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 

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

2023-10-01 Thread Oliver Webb via Toybox
--- Original Message ---
On Sunday, October 1st, 2023 at 7:50 AM, Jarno Mäkipää  
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.

> 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.

> 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 From 4117135f883cad0781d3eaa9da5ccec3c6376990 Mon Sep 17 00:00:00 2001
From: Oliver Webb 
Date: Sun, 1 Oct 2023 09:35:12 -0500
Subject: [PATCH] tests/vi.test: Added test cases for ex commands in vi

---
 tests/vi.test | 32 
 1 file changed, 32 insertions(+)

diff --git a/tests/vi.test b/tests/vi.test
index 4e84ea4c..88e07ed3 100644
--- a/tests/vi.test
+++ b/tests/vi.test
@@ -134,3 +134,35 @@ toyonly testing "insert multi yank move and push ascii" \
 # teardown
 rm in.txt cmd.txt out.txt
 
+# setup
+cp $FILES/vi/ascii.txt in.txt
+echo -e "\e:$\n:d\n:wq" > cmd.txt
+head -n3 $FILES/vi/ascii.txt > out.txt
+
+toyonly testing "delete last line ex cmd ascii" \
+  "vi -s cmd.txt in.txt 1>/dev/null 2>/dev/null && cmp in.txt out.txt && echo yes" "yes\n" "" ""
+
+# teardown
+rm in.txt cmd.txt out.txt
+
+# setup
+cp $FILES/vi/ascii.txt in.txt
+echo -e "\e:2,3d\n:wq" > cmd.txt
+echo -e "abc def hij\n" > out.txt
+
+toyonly testing "line range delete ex cmd ascii" \
+  "vi -s cmd.txt in.txt 1>/dev/null 2>/dev/null && cmp in.txt out.txt && echo yes" "yes\n" "" ""
+
+# teardown
+rm in.txt cmd.txt out.txt
+
+# setup
+cp $FILES/vi/ascii.txt in.txt
+echo -e "\e:%d\n:wq" > cmd.txt
+echo > out.txt
+
+toyonly testing "% ranges delete ex cmd ascii" \
+  "vi -s cmd.txt in.txt 1>/dev/null 2>/dev/null && cmp in.txt out.txt && echo yes" "yes\n" "" ""
+
+# teardown
+rm in.txt cmd.txt out.txt
-- 
2.34.1

___
Toybox mailing list
Toybox@lists.landley.net

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

2023-10-01 Thread Jarno Mäkipää
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. 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, 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.

If you plan to add features you could add some tests into /tests/vi.test

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

On Sun, Oct 1, 2023 at 7:55 AM Oliver Webb via Toybox
 wrote:
>
> Heya, I have been working a few days on vi.c and have managed to
> add a few things. I have a barely-working version of the "g" command
> which I have ultimately decided to omit from this patch.
>
> First thing I added was line ranges, the ability to specify
> stuff like "10,45d" and have it delete lines 10-45, and also "%"
> so you can do stuff like "%d" (and "%s" if I can get that working)
> It only supports plain numbers and doesn't do anything fancy like "+NUMBER" 
> yet.
> To get line ranges to work properly, The page has to be re-drawn or else the 
> frame
> breaks (At least while deleting lines, the only thing I can do with these 
> features as of now)
>
> Also, after some testing I realized that the line gotos that I
> implemented didn't work in some circumstances, such as at the
> top or bottom of a file and/or after moving around in a file using
> the cursor keys. I don't know why it does this... my code called
> vi_go() and after some testing, calls it in the exact same
> way that the "G" command does. I fixed this by making all line goto's run the
> "G" command and let that do whatever it does to prevent that.

This is where unit tests would be useful. To test all the corner
cases. There might be also some underlying bug relating to arrow keys
since they are handled bit different than some other commands.

>
> I have added in the support for "Go down half a page" CTRL-D,
> and added in the CTL macro after
>  mentioned it. and replaced "27" with "\e"
>
> - Oliver Webb 
> ___
> Toybox mailing list
> Toybox@lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net


-Jarno
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net