smart-insert-last-word bug in zsh 5.0.7

smart-insert-last-word bug in zsh 5.0.7

[Vincent Lefevre]

With zsh 5.0.7 under Debian (Debian package):

ypig% autoload -U smart-insert-last-word
ypig% zle -N insert-last-word smart-insert-last-word
ypig% echo abc def
abc def

Then I type:

  echo [Esc].[Left][Backspace][Ctrl-e] [Esc].

I get successively:

  echo def[ ]
  echo de[f]
  echo d[f]
  echo df[ ]
  echo df [ ]
  echo smart-insert-last-word[ ]

where [] is the position of the cursor. The second [Esc]. should have
given:

  echo df def[ ]

like without smart-insert-last-word.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: smart-insert-last-word bug in zsh 5.0.7

[Bart Schaefer]

On Jun 12, 11:16am, Vincent Lefevre wrote:
} Subject: smart-insert-last-word bug in zsh 5.0.7
}
} ypig% autoload -U smart-insert-last-word
} ypig% zle -N insert-last-word smart-insert-last-word
} ypig% echo abc def
} abc def
} 
}   echo [Esc].[Left][Backspace][Ctrl-e] [Esc].
} 
} The second [Esc]. should have given:
} 
}   echo df def[ ]
} 
} like without smart-insert-last-word.

So the problem here is that smart-insert-last-word is using the cursor
position as a clue for whether it should start the search over, or look
farther up the history.  This is still true in 5.0.8.

When you do

  [Esc].[Left][Backspace][Ctrl-e][Space]

you change the line, but then return the cursor to its previous offset.
smart-insert-last-word thinks this means it should pick up where it left
off, because neither the history number nor the cursor position have
changed.

Now that we have UNDO_CHANGE_NO, that's probably a better way to track
the state, but maybe someone can find a different problem with that?


diff --git a/Functions/Zle/smart-insert-last-word b/Functions/Zle/smart-insert-last-word
index 27b0849..5288c1d 100644
--- a/Functions/Zle/smart-insert-last-word
+++ b/Functions/Zle/smart-insert-last-word
@@ -47,13 +47,13 @@ setopt extendedglob nohistignoredups
 zle auto-suffix-retain
 
 # Not strictly necessary:
-# (($+_ilw_hist)) || integer -g _ilw_hist _ilw_count _ilw_cursor _ilw_lcursor
+# (($+_ilw_hist)) || integer -g _ilw_hist _ilw_count _ilw_changeno _ilw_lcursor
 
 integer cursor=$CURSOR lcursor=$CURSOR
 local lastcmd pattern numeric=$NUMERIC
 
 # Save state for repeated calls
-if (( HISTNO == _ilw_hist && cursor == _ilw_cursor )); then
+if (( HISTNO == _ilw_hist && UNDO_CHANGE_NO == _ilw_changeno )); then
     NUMERIC=$[_ilw_count+1]
     lcursor=$_ilw_lcursor
 else
@@ -116,4 +116,4 @@ fi
 (( NUMERIC > $#lastcmd )) && return 1
 
 LBUFFER[lcursor+1,cursor+1]=$lastcmd[-NUMERIC]
-_ilw_cursor=$CURSOR
+_ilw_changeno=$UNDO_CHANGE_NO

Re: smart-insert-last-word bug in zsh 5.0.7

[Peter Stephenson]

On Fri, 12 Jun 2015 12:12:42 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Now that we have UNDO_CHANGE_NO, that's probably a better way to track
> the state, but maybe someone can find a different problem with that?

I suppose you could do both that and cursor position, but using an
intervening change seems entirely logical if that becomes the documented way
of doing it.

pws

Re: smart-insert-last-word bug in zsh 5.0.7

[Bart Schaefer]

On Jun 13,  5:52pm, Peter Stephenson wrote:
} Subject: Re: smart-insert-last-word bug in zsh 5.0.7
}
} On Fri, 12 Jun 2015 12:12:42 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > Now that we have UNDO_CHANGE_NO, that's probably a better way to track
} > the state, but maybe someone can find a different problem with that?
} 
} I suppose you could do both that and cursor position, but using an
} intervening change seems entirely logical if that becomes the documented way
} of doing it.

After further testing, the change number doesn't quite work because it's
not updated soon enough -- the value saved at the end is never the same
as the new value on re-entry.  Also, executing "zle undo" doesn't alter
UNDO_CHANGE_NO.  So probably some test of both is necessary.

OTOH, does anyone recall why we didn't use the typical LASTWIDGET test?

diff --git a/Functions/Zle/smart-insert-last-word b/Functions/Zle/smart-insert-last-word
index 27b0849..5673136 100644
--- a/Functions/Zle/smart-insert-last-word
+++ b/Functions/Zle/smart-insert-last-word
@@ -47,13 +47,15 @@ setopt extendedglob nohistignoredups
 zle auto-suffix-retain
 
 # Not strictly necessary:
-# (($+_ilw_hist)) || integer -g _ilw_hist _ilw_count _ilw_cursor _ilw_lcursor
+# (($+_ilw_hist)) || integer -g _ilw_hist _ilw_count _ilw_cursor _ilw_lcursor _ilw_changeno
 
 integer cursor=$CURSOR lcursor=$CURSOR
 local lastcmd pattern numeric=$NUMERIC
 
 # Save state for repeated calls
-if (( HISTNO == _ilw_hist && cursor == _ilw_cursor )); then
+if (( HISTNO == _ilw_hist && cursor == _ilw_cursor &&
+      UNDO_CHANGE_NO == _ilw_changeno ))
+then
     NUMERIC=$[_ilw_count+1]
     lcursor=$_ilw_lcursor
 else
@@ -61,7 +63,8 @@ else
     _ilw_lcursor=$lcursor
 fi
 # Handle the up to three arguments of .insert-last-word
-if (( $+1 )); then
+if (( $+1 ))
+then
     if (( $+3 )); then
 	((NUMERIC = -($1)))
     else
@@ -116,4 +119,11 @@ fi
 (( NUMERIC > $#lastcmd )) && return 1
 
 LBUFFER[lcursor+1,cursor+1]=$lastcmd[-NUMERIC]
-_ilw_cursor=$CURSOR
+
+if (( _ilw_cursor != CURSOR ))
+then
+    _ilw_cursor=$CURSOR
+
+    # UNDO_CHANGE_NO increments after we return, but why by two?
+    _ilw_changeno=$((UNDO_CHANGE_NO+2))
+fi

-- 
Barton E. Schaefer

Re: smart-insert-last-word bug in zsh 5.0.7

[Bart Schaefer]

On Jun 13, 11:23am, Bart Schaefer wrote:
}
} After further testing, the change number doesn't quite work because it's
} not updated soon enough -- the value saved at the end is never the same
} as the new value on re-entry.  Also, executing "zle undo" doesn't alter
} UNDO_CHANGE_NO.  So probably some test of both is necessary.

Alas, this is still not sufficient.  The ugly bit is here:

} +if (( _ilw_cursor != CURSOR ))
} +then
} +    _ilw_cursor=$CURSOR
} +
} +    # UNDO_CHANGE_NO increments after we return, but why by two?
} +    _ilw_changeno=$((UNDO_CHANGE_NO+2))
} +fi

If two consecutive last words have the same number of characters,
_ilw_cursor == CURSOR and then _ilw_changeno is not updated.  But if
_ilw_changeno is updated unconditionally, then it becomes wrong if
UNDO_CHANGE_NO is not incremented, which might happen if two
consecutive last words are the same string (a replacement of a string
by itself is not treated as an undo-able change).

The answer seems to be to force an undo point with split-undo.


diff --git a/Functions/Zle/smart-insert-last-word b/Functions/Zle/smart-insert-last-word
index 27b0849..67adea7 100644
--- a/Functions/Zle/smart-insert-last-word
+++ b/Functions/Zle/smart-insert-last-word
@@ -47,13 +47,15 @@ setopt extendedglob nohistignoredups
 zle auto-suffix-retain
 
 # Not strictly necessary:
-# (($+_ilw_hist)) || integer -g _ilw_hist _ilw_count _ilw_cursor _ilw_lcursor
+# (($+_ilw_hist)) || integer -g _ilw_hist _ilw_count _ilw_cursor _ilw_lcursor _ilw_changeno
 
 integer cursor=$CURSOR lcursor=$CURSOR
 local lastcmd pattern numeric=$NUMERIC
 
 # Save state for repeated calls
-if (( HISTNO == _ilw_hist && cursor == _ilw_cursor )); then
+if (( HISTNO == _ilw_hist && cursor == _ilw_cursor &&
+      UNDO_CHANGE_NO == _ilw_changeno ))
+then
     NUMERIC=$[_ilw_count+1]
     lcursor=$_ilw_lcursor
 else
@@ -61,7 +63,8 @@ else
     _ilw_lcursor=$lcursor
 fi
 # Handle the up to three arguments of .insert-last-word
-if (( $+1 )); then
+if (( $+1 ))
+then
     if (( $+3 )); then
 	((NUMERIC = -($1)))
     else
@@ -117,3 +120,6 @@ fi
 
 LBUFFER[lcursor+1,cursor+1]=$lastcmd[-NUMERIC]
 _ilw_cursor=$CURSOR
+
+# This is necessary to update UNDO_CHANGE_NO immediately
+zle split-undo && _ilw_changeno=$UNDO_CHANGE_NO