Skip to content

Commit

Permalink
acme: Avoid wrapping or raising errors while evaluating addresses
Browse files Browse the repository at this point in the history
Fixes 9fans#122.

As reported in 9fans#122, file:1:1 moves to the end of the file,
and file:1:2 fails with “address out of range”.

I’ll use file:2:3 as an example so we can tell the line and column number apart.

What’s happening is this:
plumb/basic matches 2:3 using twocolonaddr (from plumb/fileaddr),
then sets addr to `2-9fans#1+9fans#3`
(the 1 is constant and was introduced because column numbers are 1-based).
Acme interprets this in three steps:
1) find the range (q0, q1) that contains line 2
2) create the range (q2, q2) where q2 = q0 - 1
3) create the range (q3, q3) where q3 = q2 + 3

The second step has a branch where if q0 == 0 and 1 > 0
(remember that 1 is constant and comes form plumb/basic),
q0 is set to _the end of the file_.
This makes addressing things at the end of the file easier.

The problem then is that if we select line 1,
which starts at the beginning of the file,
q0 is always 0 and the branch in step 2) will _always_ be used.
1:1 is interpreted as 1-9fans#1+9fans#1 which starts at 0, wraps around to the end of the file, then moves 1 character backwards and then forwards again, ending at the end of the file.
1:2 is interpretes as 1-9fans#1+9fans#2 which starts at 0, wraps around to the end od the file, then moves 1 character backwards and tries moving 2 characters forwards beyond the end of the file, resulting in the out of range error.

I can think of several ways to solve or work around this problem:

1)	Let column numbers start from 0.
	Plumb/basic would use `-#0` when setting addr,
	which does not move the left side of the range and would not wrap around.
	This would require no changes to the code
	but could break compatibility with people’s setups.

2)	Create a new address notation for this use case, like `line:column`.
	The problem is essentially that the plumbing rule
	transforms something that declares _intent_
	into a sequence of operations that _should_ match that intent
	but which can run into unexpected conditions when interpreted.
	New notation would preserve the intention and could be interpreted accordingly.
	This would require changes to every program that interprets addr,
	so at least acme and sam, and to the plumbing rules.

3)	Let forward character motion wrap around to the beginning of the file.
	The rule in plumb/basic assumes that moving backwards and then forwards
	by 1 character ends at the start of the previous selection.
	Letting forward motion wrap around restores this property
	when interpreting addresses.
	This would require changes to addr.c,
	but I’m not sure whether there are other pieces of software or people’s setups
	that assume that forward motion does _not_ wrap around.
	It also seems like it would blur the conceptual lines
	between the beginning and the end of a file, which seems undesirable.

4)	Require empty ranges to enable wrapping, delay out of range errors.
	The wrapping currently occurs whenever the current range starts at position 0,
	although (what I imagine to be) the most common use case is negative motion
	starting from the initial empty range (0,0).
	If we restrict wrapping so it happens only when the range is (0,0),
	the motion sequence mentioned above would have an intermediate invalid state,
	a range (-1,-1), which would result in an out of range error.
	If we skip generating that error we will arrive at the correct position.
	Just skipping the error handling here would mean that
	textsetselect would handle those errors,
	and it currently adjusts ranges instead of generating error messages.
	If that is undesirable, we could add error handling at the end of address.

	Instead of wrapping from (0,0), wrapping could also require a flag to number
	that address initially sets to TRUE but sets to FALSE once it has left (0,0).
	This seems more explicit about when it would allow wrapping
	but it would prevent addresses like file:1-0-9fans#1 from wrapping to the end.

This change implements 4).
  • Loading branch information
mkhl committed Feb 1, 2018
1 parent da8a485 commit 158d707
Showing 1 changed file with 1 addition and 3 deletions.
4 changes: 1 addition & 3 deletions src/cmd/acme/addr.c
Expand Up @@ -79,12 +79,10 @@ number(uint showerr, Text *t, Range r, int line, int dir, int size, int *evalp)
if(dir == Fore)
line = r.q1+line;
else if(dir == Back){
if(r.q0==0 && line>0)
if(r.q0==0 && r.q1==0 && line>0)
r.q0 = t->file->b.nc;
line = r.q0 - line;
}
if(line<0 || line>t->file->b.nc)
goto Rescue;
*evalp = TRUE;
return range(line, line);
}
Expand Down

0 comments on commit 158d707

Please sign in to comment.