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

[Daniel Hahler <genml+zsh-workers@thequod.de>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Oliver,

this patch breaks the following widget I am using (via http://chneukirchen.org/blog/archive/2013/03/10-fresh-zsh-tricks-you-may-not-know.html):

autoload -Uz narrow-to-region
function _history-incremental-preserving-pattern-search-backward
{
  local state
  MARK=CURSOR  # magick, else multiple ^R don't work
  narrow-to-region -p "$LBUFFER${BUFFER:+>>}" -P "${BUFFER:+<<}$RBUFFER" -S state
  zle end-of-history
  zle history-incremental-pattern-search-backward
  narrow-to-region -R state
}
zle -N _history-incremental-preserving-pattern-search-backward
bindkey "^R" _history-incremental-preserving-pattern-search-backward
bindkey -M isearch "^R" history-incremental-pattern-search-backward
bindkey "^S" history-incremental-pattern-search-forward


After ^R, pressing Enter will add the same line to the end again.
This also happens when pressing a cursor key etc.

TEST CASE:
% echo foo
foo
% ^Rfoo^M
fooecho foo

Commit: 832130c57dedc532191512045096180657a049f3


Regards,
Daniel.

On 12.08.2015 19:00, Oliver Kiddle wrote:
> 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;
>  }
>  
>  /**/
> 
-----BEGIN PGP SIGNATURE-----

iEYEARECAAYFAlXSTt8ACgkQfAK/hT/mPgBQFwCfbI+trLonGkP6845YlVrNZPoF
XM4AoKynpdr5Yf7wTReGSog4HMTdjAOl
=bq0S
-----END PGP SIGNATURE-----