PATCH: Re: Undo and narrow-to-region (was ...)

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

Here is the narrow-to-region with support for undo limits along with
a related change to the undo mechanism.

It'd be good if this gets some wider testing because I only use
narrow-to-region in limited ways. This is showing up issues with
$UNDO_CHANGE_NO and zle undo to undo multiple changes.

The change to zle_utils.c here is for $UNDO_CHANGE_NO. 
If you consider this function:
  undostuff() {
    LBUFFER+='a'
    LBUFFER+='b'
    zle split-undo
    LBUFFER+='c'
    _undo_here=$UNDO_CHANGE_NO
    LBUFFER+='d'
  }

A subsequent zle undo $_undo_here should take you back to 'abc' while a
default undo should skip straight to 'ab' without stopping at 'abc'.

The following adds a call to mkundoent() when $UNDO_CHANGE_NO is
referenced so you get a clear change number marking the current
state. I think the result is simpler than the current tricks with
undo_set_by_variable.

This is still not perfect with history changes. I'm not really happy
with the way history line changes are managed in the undo system in
general. The old fix in 10328 seems rather ugly. It might be better to
create full undo entries for history changes though that would need
hacks to ensure that multiple history changes are undone in a single
step.

This patch also replaces 35935.

Oliver

diff --git a/Functions/Zle/narrow-to-region b/Functions/Zle/narrow-to-region
index 293f89b..0ef28a8 100644
--- a/Functions/Zle/narrow-to-region
+++ b/Functions/Zle/narrow-to-region
@@ -26,11 +26,13 @@
 #      statevar may not begin with the prefix "_ntr_" which is reserved for
 #      parameters within narrow-to-region.
 
-emulate -L zsh
-setopt extendedglob
+# set the minimum of options to avoid changing behaviour away from
+# user preferences from within recursive-edit
+setopt localoptions noshwordsplit noksharrays
 
-local _ntr_lbuf_return _ntr_rbuf_return
+local _ntr_newbuf _ntr_lbuf_return _ntr_rbuf_return
 local _ntr_predisplay=$PREDISPLAY _ntr_postdisplay=$POSTDISPLAY
+integer _ntr_savelim=UNDO_LIMIT_NO _ntr_changeno
 integer _ntr_start _ntr_end _ntr_swap _ntr_cursor=$CURSOR _ntr_mark=$MARK
 integer _ntr_stat
 
@@ -61,7 +63,7 @@ done
 (( OPTIND > 1 )) && shift $(( OPTIND - 1 ))
 
 if [[ $_ntr_restore = _ntr_* || $_ntr_save = _ntr_* ||
-      $_ntr_lbuf_return = _ntr_* || $ntr_rbuf_return = _ntr_* ]]; then
+      $_ntr_lbuf_return = _ntr_* || $_ntr_rbuf_return = _ntr_* ]]; then
   zle -M "$0: _ntr_ prefix is reserved" >&2
   return 1
 fi
@@ -86,30 +88,34 @@ if [[ -n $_ntr_save || -z $_ntr_restore ]]; then
     _ntr_end=_ntr_swap
   fi
 
-  (( _ntr_end++, _ntr_cursor -= _ntr_start, _ntr_mark -= _ntr_start ))
+  (( _ntr_cursor -= _ntr_start, _ntr_mark -= _ntr_start ))
   
   _ntr_lbuffer=${BUFFER[1,_ntr_start]}
   if [[ -z $_ntr_usepretext || ( -n $_ntr_nonempty && -z $_ntr_lbuffer ) ]]
   then
     _ntr_pretext=$_ntr_lbuffer
   fi
-  _ntr_rbuffer=${BUFFER[_ntr_end,-1]}
+  _ntr_rbuffer=${BUFFER[_ntr_end+1,-1]}
   if [[ -z $_ntr_useposttext || ( -n $_ntr_nonempty && -z $_ntr_rbuffer ) ]]
   then
     _ntr_posttext=$_ntr_rbuffer
   fi
+  _ntr_changeno=$UNDO_CHANGE_NO
   PREDISPLAY="$_ntr_predisplay$_ntr_pretext"
   POSTDISPLAY="$_ntr_posttext$_ntr_postdisplay"
 
   if [[ -n $_ntr_save ]]; then
     builtin typeset -ga $_ntr_save
     set -A $_ntr_save "${_ntr_predisplay}" "${_ntr_postdisplay}" \
-	"${_ntr_lbuffer}" "${_ntr_rbuffer}" || return 1
+	"${_ntr_savelim}" "${_ntr_changeno}" \
+	"${_ntr_start}" "${_ntr_end}" || return 1
   fi
 
-  BUFFER=${BUFFER[_ntr_start+1,_ntr_end-1]}
+  BUFFER=${BUFFER[_ntr_start+1,_ntr_end]}
   CURSOR=$_ntr_cursor
   MARK=$_ntr_mark
+  zle split-undo
+  UNDO_LIMIT_NO=$UNDO_CHANGE_NO
 fi
 
 if [[ -z $_ntr_save && -z $_ntr_restore ]]; then
@@ -126,18 +132,22 @@ if [[ -n $_ntr_restore || -z $_ntr_save ]]; then
   if [[ -n $_ntr_restore ]]; then
     if ! { _ntr_predisplay="${${(@P)_ntr_restore}[1]}"
            _ntr_postdisplay="${${(@P)_ntr_restore}[2]}"
-           _ntr_lbuffer="${${(@P)_ntr_restore}[3]}"
-           _ntr_rbuffer="${${(@P)_ntr_restore}[4]}" }; then
+	   _ntr_savelim="${${(@P)_ntr_restore}[3]}"
+	   _ntr_changeno="${${(@P)_ntr_restore}[4]}"
+	   _ntr_start="${${(@P)_ntr_restore}[5]}"
+	   _ntr_end="${${(@P)_ntr_restore}[6]}" }; then
       zle -M Failed. >&2
       return 1
     fi
   fi
 
+  _ntr_newbuf="$BUFFER"
+  zle undo $_ntr_changeno
   PREDISPLAY=$_ntr_predisplay
   POSTDISPLAY=$_ntr_postdisplay
-  LBUFFER="$_ntr_lbuffer$BUFFER"
-  RBUFFER="$_ntr_rbuffer"
-  MARK=${#_ntr_lbuffer}
+  BUFFER[_ntr_start+1,_ntr_end]="$_ntr_newbuf"
+  (( MARK = _ntr_start, CURSOR = _ntr_start + ${#_ntr_newbuf} ))
+  UNDO_LIMIT_NO=_ntr_savelim
 fi
 
 return $_ntr_stat
diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
index 8b55403..d1d3206 100644
--- a/Src/Zle/zle_utils.c
+++ b/Src/Zle/zle_utils.c
@@ -1409,10 +1409,6 @@ zlong undo_changeno;
 
 zlong undo_limitno;
 
-/* If non-zero, the last increment to undo_changeno was for the variable */
-
-static int undo_set_by_variable;
-
 /**/
 void
 initundo(void)
@@ -1423,7 +1419,6 @@ initundo(void)
     curchange->del = curchange->ins = NULL;
     curchange->dell = curchange->insl = 0;
     curchange->changeno = undo_changeno = undo_limitno = 0;
-    undo_set_by_variable = 0;
     lastline = zalloc((lastlinesz = linesz) * ZLE_CHAR_SIZE);
     ZS_memcpy(lastline, zleline, (lastll = zlell));
     lastcs = zlecs;
@@ -1549,7 +1544,6 @@ mkundoent(void)
 	ch->prev = NULL;
     }
     ch->changeno = ++undo_changeno;
-    undo_set_by_variable = 0;
     endnextchanges = ch;
 }
 
@@ -1584,14 +1578,13 @@ undo(char **args)
 	struct change *prev = curchange->prev;
 	if(!prev)
 	    return 1;
-	if (prev->changeno < last_change)
+	if (prev->changeno <= last_change)
 	    break;
-	if (prev->changeno < undo_limitno && !*args)
+	if (prev->changeno <= undo_limitno && !*args)
 	    return 1;
-	if (unapplychange(prev))
-	    curchange = prev;
-	else
-	    break;
+	if (!unapplychange(prev) && last_change >= 0)
+	    unapplychange(prev);
+	curchange = prev;
     } while (last_change >= (zlong)0 || (curchange->flags & CH_PREV));
     setlastline();
     return 0;
@@ -1741,18 +1734,10 @@ zlecallhook(char *name, char *arg)
 zlong
 get_undo_current_change(UNUSED(Param pm))
 {
-    if (undo_set_by_variable) {
-	/* We were the last to increment this, doesn't need another one. */
-	return undo_changeno;
-    }
-    undo_set_by_variable = 1;
-    /*
-     * Increment the number in case a change is in progress;
-     * we don't want to back off what's already been done when
-     * we return to this change number.  This eliminates any
-     * problem about the point where a change is numbered
-     */
-    return ++undo_changeno;
+    /* add entry for any pending changes */
+    mkundoent();
+    setlastline();
+    return undo_changeno;
 }
 
 /**/