* This widget implementation feels a bit clunky (edit-quoted-word) @ 2015-06-20 9:16 Mikael Magnusson 2015-06-20 17:06 ` Peter Stephenson 2015-06-20 17:21 ` This widget implementation feels a bit clunky (edit-quoted-word) Bart Schaefer 0 siblings, 2 replies; 19+ messages in thread From: Mikael Magnusson @ 2015-06-20 9:16 UTC (permalink / raw) To: zsh workers autoload -U narrow-to-region function _narrow_to_region_marked() { #I had this function since before local right local left local OLDMARK=MARK local wasregion=1 if ((REGION_ACTIVE == 0)); then MARK=CURSOR wasregion=0 fi REGION_ACTIVE=0 if ((MARK < CURSOR)); then left="$LBUFFER[0,$((MARK-CURSOR-1))]" right="$RBUFFER" else left="$LBUFFER" right="$BUFFER[$((MARK+1)),-1]" fi narrow-to-region -p "$left>>|" -P "|<<$right" _line_redraw #update highlight stuff doesn't happen here otherwise MARK=OLDMARK if ((wasregion)); then REGION_ACTIVE=1 fi } function _edit_quoted_word() { local -a reply local REPLY REPLY2 len _split_shell_arguments_intuitive local STARTPOS ENDPOS local OLDCURSOR=$CURSOR OLDMARK=$MARK wasregion=$REGION_ACTIVE STARTPOS=${#${(j::)reply[1,REPLY-1]}} reply[REPLY]=${(Q)reply[REPLY]} ENDPOS=${#${(j::)reply[1,REPLY]}} CURSOR=$STARTPOS MARK=$ENDPOS REGION_ACTIVE=1 BUFFER=${(j::)reply} len=$#BUFFER _narrow_to_region_marked CURSOR=$STARTPOS MARK=$((ENDPOS+$#BUFFER-len)) zle quote-region #the following line is also my quote-unquote-word widget modify-current-argument '${(q-)${(Q)ARG}}' CURSOR=$OLDCURSOR MARK=$OLDMARK REGION_ACTIVE=$wasregion } zle -N edit-quoted-word _edit_quoted_word bindkey '^_q' edit-quoted-word autoload -U modify-current-argument autoload -U split-shell-arguments function _split_shell_arguments_intuitive() { #Had this since before too split-shell-arguments #have to duplicate some of modify-current-argument to get the word #_under_ the cursor, not after. setopt localoptions noksharrays multibyte if (( REPLY > 1 )); then if (( REPLY & 1 )); then (( REPLY-- )) fi fi } I'm not exactly sure how often I would use this, but I had the thought and set off on an adventure. This lets you be on a thing like % echo I\''d (like) to "have\" some better example?' turns into % echo >>|I'd (like) to "have\" some better example?|<< which is then somewhat easier to edit, then you maybe type this instead % echo >>|I'd (like) to "have\" '${(q-)${(Q)ARG}}' some better example?|<< after pressing enter, it turns back into % echo I\''d (like) to "have\" ''${(q-)${(Q)ARG}}'' some better example?' I feel like it's unnecessarily hard to get the resulting text from a narrow-to-region session. It would be nice if quote-region let you specify the type of quoting. Urk? -- Mikael Magnusson ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: This widget implementation feels a bit clunky (edit-quoted-word) 2015-06-20 9:16 This widget implementation feels a bit clunky (edit-quoted-word) Mikael Magnusson @ 2015-06-20 17:06 ` Peter Stephenson 2015-06-21 7:09 ` Mikael Magnusson 2015-06-20 17:21 ` This widget implementation feels a bit clunky (edit-quoted-word) Bart Schaefer 1 sibling, 1 reply; 19+ messages in thread From: Peter Stephenson @ 2015-06-20 17:06 UTC (permalink / raw) To: zsh workers On Sat, 20 Jun 2015 11:16:20 +0200 Mikael Magnusson <mikachu@gmail.com> wrote: > I feel like it's unnecessarily hard to get the resulting text from a > narrow-to-region session. Would you want something like this? diff --git a/Functions/Zle/narrow-to-region b/Functions/Zle/narrow-to-region index 86fd7ac..d449540 100644 --- a/Functions/Zle/narrow-to-region +++ b/Functions/Zle/narrow-to-region @@ -11,6 +11,8 @@ # Either or both may be empty. # -n Only replace the text before or after the region with # the -p or -P options if the text was not empty. +# -l lbufvar ) $lbufvar and $rbufvar will contain the value of $LBUFFER and +# -r rbufvar ) $RBUFFER resulting from any recursive edit (i.e. not with -S or -R) # -S statevar # -R statevar # Save or restore the state in/from the parameter named statevar. In @@ -28,16 +30,20 @@ integer _ntr_start _ntr_end _ntr_swap _ntr_cursor=$CURSOR _ntr_mark=$MARK integer _ntr_stat local _ntr_opt _ntr_pretext _ntr_posttext _ntr_usepretext _ntr_useposttext -local _ntr_nonempty _ntr_save _ntr_restore +local _ntr_nonempty _ntr_save _ntr_restore _ntr_lbuffer _ntr_rbuffer -while getopts "np:P:R:S:" _ntr_opt; do +while getopts "l:np:P:r:R:S:" _ntr_opt; do case $_ntr_opt in + (l) _ntr_lbuffer=$OPTARG + ;; (n) _ntr_nonempty=1 ;; (p) _ntr_pretext=$OPTARG _ntr_usepretext=1 ;; (P) _ntr_posttext=$OPTARG _ntr_useposttext=1 ;; + (r) _ntr_rbuffer=$OPTARG + ;; (R) _ntr_restore=$OPTARG ;; (S) _ntr_save=$OPTARG @@ -101,6 +107,9 @@ fi if [[ -z $_ntr_save && -z $_ntr_restore ]]; then zle recursive-edit _ntr_stat=$? + + [[ -n $_ntr_lbuffer ]] && eval "${_ntr_lbuffer}=\${LBUFFER}" + [[ -n $_ntr_rbuffer ]] && eval "${_ntr_rbuffer}=\${RBUFFER}" fi if [[ -n $_ntr_restore || -z $_ntr_save ]]; then ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: This widget implementation feels a bit clunky (edit-quoted-word) 2015-06-20 17:06 ` Peter Stephenson @ 2015-06-21 7:09 ` Mikael Magnusson 2015-06-21 17:17 ` Peter Stephenson 2015-06-22 0:18 ` PATCH: Document narrow-to-region -l and -r Mikael Magnusson 0 siblings, 2 replies; 19+ messages in thread From: Mikael Magnusson @ 2015-06-21 7:09 UTC (permalink / raw) To: Peter Stephenson, Bart Schaefer; +Cc: zsh workers On Sat, Jun 20, 2015 at 7:06 PM, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote: > On Sat, 20 Jun 2015 11:16:20 +0200 > Mikael Magnusson <mikachu@gmail.com> wrote: >> I feel like it's unnecessarily hard to get the resulting text from a >> narrow-to-region session. > > Would you want something like this? That does look very promising, yeah. :) I suppose I should volunteer for writing the doc patch for this. On Sat, Jun 20, 2015 at 7:21 PM, Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jun 20, 11:16am, Mikael Magnusson wrote: > } > } I feel like it's unnecessarily hard to get the resulting text from a > } narrow-to-region session. > > Hmm. If you used "narrow-to-region -S state ..." then I think the > resulting text would be $BUFFER between the exit from recursive-edit > and the call to "narrow-to-region -R state" ? > > Except that I'm having a little trouble with -S / -R. The value of > $MARK never seems to be correctly restored. Is it supposed to be? I'll admit I didn't actually look at the impl of the widget, only the documentation. > } It would be nice if quote-region let you specify the type of quoting. > } Urk? > > The difficulty there is that it's a widget so there's no direct way to > pass the type of quote. I suppose you could use $NUMERIC. I thought widgets can take an arbitrary amount of arguments, although it is true that almost none do. The newly added bracketed-paste takes an argument that names the parameter to store pasted text in for example. If you mean that you can't do it directly via bindkey then yeah, that's true, you'd still need a wrapper widget. For my use-case I could put the argument right in with the zle call though. (But with PWS' suggested change, I can just insert (q-)LBUFFER/RBUFFER directly, I think). -- Mikael Magnusson ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: This widget implementation feels a bit clunky (edit-quoted-word) 2015-06-21 7:09 ` Mikael Magnusson @ 2015-06-21 17:17 ` Peter Stephenson 2015-06-22 2:26 ` Mikael Magnusson 2015-06-22 0:18 ` PATCH: Document narrow-to-region -l and -r Mikael Magnusson 1 sibling, 1 reply; 19+ messages in thread From: Peter Stephenson @ 2015-06-21 17:17 UTC (permalink / raw) To: zsh workers On Sun, 21 Jun 2015 09:09:29 +0200 Mikael Magnusson <mikachu@gmail.com> wrote: > On Sat, Jun 20, 2015 at 7:06 PM, Peter Stephenson > <p.w.stephenson@ntlworld.com> wrote: > > On Sat, 20 Jun 2015 11:16:20 +0200 > > Mikael Magnusson <mikachu@gmail.com> wrote: > >> I feel like it's unnecessarily hard to get the resulting text from a > >> narrow-to-region session. > > > > Would you want something like this? > > That does look very promising, yeah. :) > I suppose I should volunteer for writing the doc patch for this. I've remembered to commit it on the master branch. pws ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: This widget implementation feels a bit clunky (edit-quoted-word) 2015-06-21 17:17 ` Peter Stephenson @ 2015-06-22 2:26 ` Mikael Magnusson 0 siblings, 0 replies; 19+ messages in thread From: Mikael Magnusson @ 2015-06-22 2:26 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh workers On Sun, Jun 21, 2015 at 7:17 PM, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote: > On Sun, 21 Jun 2015 09:09:29 +0200 > Mikael Magnusson <mikachu@gmail.com> wrote: >> On Sat, Jun 20, 2015 at 7:06 PM, Peter Stephenson >> <p.w.stephenson@ntlworld.com> wrote: >> > On Sat, 20 Jun 2015 11:16:20 +0200 >> > Mikael Magnusson <mikachu@gmail.com> wrote: >> >> I feel like it's unnecessarily hard to get the resulting text from a >> >> narrow-to-region session. >> > >> > Would you want something like this? >> >> That does look very promising, yeah. :) >> I suppose I should volunteer for writing the doc patch for this. > > I've remembered to commit it on the master branch. This feels more reasonable now (some of the improvements might be applied to the previous version too, so not an entirely fair comparison), (the business with the :+ is because if you delete the whole string, ${(q-)lbuf} when lbuf is empty results in '', and so does rbuf, thus concatenated to '''' which with rc_quotes enabled leaves a single ' that should not be there). function _edit_quoted_word() { local -a reply local REPLY REPLY2 len left right lbuf rbuf _split_shell_arguments_intuitive left=${(j::)reply[1,REPLY-1]} right=${(j::)reply[REPLY+1,-1]} (( CURSOR-=$#left )) BUFFER=${(Q)reply[REPLY]} REGION_ACTIVE=0 narrow-to-region -l lbuf -r rbuf -p "$left>>|" -P "|<<$right" 0 $#BUFFER LBUFFER=$left${lbuf:+${(q-)lbuf}} RBUFFER=${rbuf:+${(q-)rbuf}}$right } zle -N _edit_quoted_word bindkey '^_q' _edit_quoted_word autoload -U split-shell-arguments function _split_shell_arguments_intuitive() { split-shell-arguments # borrowed logic from modify-current-argument to get word under cursor (( REPLY > 1 && REPLY & 1 )) && (( REPLY-- )) } -- Mikael Magnusson ^ permalink raw reply [flat|nested] 19+ messages in thread
* PATCH: Document narrow-to-region -l and -r. 2015-06-21 7:09 ` Mikael Magnusson 2015-06-21 17:17 ` Peter Stephenson @ 2015-06-22 0:18 ` Mikael Magnusson 1 sibling, 0 replies; 19+ messages in thread From: Mikael Magnusson @ 2015-06-22 0:18 UTC (permalink / raw) To: zsh-workers --- Doc/Zsh/contrib.yo | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Doc/Zsh/contrib.yo b/Doc/Zsh/contrib.yo index 323bf0f..09ac5c8 100644 --- a/Doc/Zsh/contrib.yo +++ b/Doc/Zsh/contrib.yo @@ -2272,7 +2272,8 @@ tindex(narrow-to-region) tindex(narrow-to-region-invisible) redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ @ @ @ @ @ @ @ @ @ @ @ @ ))ifnztexi( ))) xitem(tt(narrow-to-region )[ tt(-p) var(pre) ] [ tt(-P) var(post) ]) -xitem(SPACES()[ tt(-S) var(statepm) | tt(-R) var(statepm) ] [ tt(-n) ] [ var(start) var(end) ]) +xitem(SPACES()[ tt(-S) var(statepm) | tt(-R) var(statepm) | [ tt(-l) var(lbufvar) ] [ tt(-r) var(rbufvar) ] ]) +xitem(SPACES()[ tt(-n) ] [ var(start) var(end) ]) item(tt(narrow-to-region-invisible))( Narrow the editable portion of the buffer to the region between the cursor and the mark, which may be in either order. The region may not be empty. @@ -2308,9 +2309,15 @@ parameter, except that parameters beginning with the prefix tt(_ntr_) are reserved for use within tt(narrow-to-region). Typically the parameter will be local to the calling function. +The options tt(-l) var(lbufvar) and tt(-r) var(rbufvar) may be used to +specify parameters where the widget will store the resulting text from +the operation. The parameter var(lbufvar) will contain var(LBUFFER) +and var(rbufvar) will contain var(RBUFFER). Neither of these two options +may be used with tt(-S) or tt(-R). + tt(narrow-to-region-invisible) is a simple widget which calls tt(narrow-to-region) with arguments which replace any text outside the -region with `tt(...)'. +region with `tt(...)'. It does not take any arguments. The display is restored (and the widget returns) upon any zle command which would usually cause the line to be accepted or aborted. Hence an -- 2.4.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: This widget implementation feels a bit clunky (edit-quoted-word) 2015-06-20 9:16 This widget implementation feels a bit clunky (edit-quoted-word) Mikael Magnusson 2015-06-20 17:06 ` Peter Stephenson @ 2015-06-20 17:21 ` Bart Schaefer 2015-07-18 23:42 ` PATCH: narrow-to-region (was Re: This widget implementation feels a bit clunky) Bart Schaefer 1 sibling, 1 reply; 19+ messages in thread From: Bart Schaefer @ 2015-06-20 17:21 UTC (permalink / raw) To: zsh workers On Jun 20, 11:16am, Mikael Magnusson wrote: } } I feel like it's unnecessarily hard to get the resulting text from a } narrow-to-region session. Hmm. If you used "narrow-to-region -S state ..." then I think the resulting text would be $BUFFER between the exit from recursive-edit and the call to "narrow-to-region -R state" ? Except that I'm having a little trouble with -S / -R. The value of $MARK never seems to be correctly restored. Is it supposed to be? } It would be nice if quote-region let you specify the type of quoting. } Urk? The difficulty there is that it's a widget so there's no direct way to pass the type of quote. I suppose you could use $NUMERIC. ^ permalink raw reply [flat|nested] 19+ messages in thread
* PATCH: narrow-to-region (was Re: This widget implementation feels a bit clunky) 2015-06-20 17:21 ` This widget implementation feels a bit clunky (edit-quoted-word) Bart Schaefer @ 2015-07-18 23:42 ` Bart Schaefer 2015-07-19 3:55 ` Oliver Kiddle 0 siblings, 1 reply; 19+ messages in thread From: Bart Schaefer @ 2015-07-18 23:42 UTC (permalink / raw) To: zsh workers On Jun 20, 10:21am, Bart Schaefer wrote: } } [...] If you used "narrow-to-region -S state ..." then I think the } resulting text would be $BUFFER between the exit from recursive-edit } and the call to "narrow-to-region -R state" ? } } Except that I'm having a little trouble with -S / -R. The value of } $MARK never seems to be correctly restored. Is it supposed to be? What was happening: In order to narrow the editable buffer to just the current region, the lesser of MARK and CURSOR is calculated and assigned to _ntr_start. Then the leading $_ntr_start characters are removed from BUFFER and MARK and CURSOR are shifted to correspond, so one or the other of them always ends up being set to 0. Upon exit from recursive-edit (or calling "narrow-to-region -R state") the values of LBUFFER and RBUFFER are reset, which automatically moves CURSOR to the beginning of the new RBUFFER, but does not change MARK. This guarantees that MARK will be wrong, because it will either be left as zero, or at a now-irrelevant position in the middle of the buffer, possibly even past the end of it. So this patch: - cleans up white space in the documentary comment - checks that the -l and -r buffer variables also don't use _ntr_ - consistently uses "zle -M ... >&2" even though I don't know why - does away with a bunch of unnecessary "eval"s - declares the -l -r and -S variables with "typeset -g" - forces MARK and CURSOR to point to opposite ends of the region after the function returns, which is really the point of all this diff --git a/Functions/Zle/narrow-to-region b/Functions/Zle/narrow-to-region index 8b88da4..293f89b 100644 --- a/Functions/Zle/narrow-to-region +++ b/Functions/Zle/narrow-to-region @@ -5,21 +5,26 @@ # Optionally accepts exactly two arguments, which are used instead of # $CURSOR and $MARK as limits to the range. # +# Upon exit, $MARK is always the start of the edited range and $CURSOR +# the end of the range, even if they began in the opposite order. +# # Other options: -# -p pretext show `pretext' instead of the buffer text before the region. -# -P posttext show `posttext' instead of the buffer text after the region. -# Either or both may be empty. +# -p pretext show "pretext" instead of the buffer text before the region. +# -P posttext show "posttext" instead of the buffer text after the region. +# Either or both may be empty. # -n Only replace the text before or after the region with # the -p or -P options if the text was not empty. -# -l lbufvar ) $lbufvar and $rbufvar will contain the value of $LBUFFER and -# -r rbufvar ) $RBUFFER resulting from any recursive edit (i.e. not with -S or -R) +# -l lbufvar $lbufvar is assigned the value of $LBUFFER and +# -r rbufvar $rbufvar is assigned the value of $RBUFFER +# from any recursive edit (i.e. not with -S or -R). Neither +# lbufvar nor rbufvar may begin with the prefix "_ntr_". # -S statevar # -R statevar -# Save or restore the state in/from the parameter named statevar. In -# either case no recursive editing takes place; this will typically be -# done within the calling function between calls with -S and -R. The -# statevar may not begin with the prefix _ntr_ which is reserved for -# parameters within narrow-to-region. +# Save or restore the state in/from the parameter named statevar. In +# either case no recursive editing takes place; this will typically be +# done within the calling function between calls with -S and -R. The +# statevar may not begin with the prefix "_ntr_" which is reserved for +# parameters within narrow-to-region. emulate -L zsh setopt extendedglob @@ -55,7 +60,8 @@ while getopts "l:np:P:r:R:S:" _ntr_opt; do done (( OPTIND > 1 )) && shift $(( OPTIND - 1 )) -if [[ $_ntr_restore = _ntr_* || $_ntr_save = _ntr_* ]]; then +if [[ $_ntr_restore = _ntr_* || $_ntr_save = _ntr_* || + $_ntr_lbuf_return = _ntr_* || $ntr_rbuf_return = _ntr_* ]]; then zle -M "$0: _ntr_ prefix is reserved" >&2 return 1 fi @@ -64,7 +70,7 @@ if [[ -n $_ntr_save || -z $_ntr_restore ]]; then if (( $# )); then if (( $# != 2 )); then - zle -M "$0: supply zero or two arguments" + zle -M "$0: supply zero or two arguments" >&2 return 1 fi _ntr_start=$1 @@ -94,39 +100,44 @@ if [[ -n $_ntr_save || -z $_ntr_restore ]]; then fi PREDISPLAY="$_ntr_predisplay$_ntr_pretext" POSTDISPLAY="$_ntr_posttext$_ntr_postdisplay" - BUFFER=${BUFFER[_ntr_start+1,_ntr_end-1]} - CURSOR=$_ntr_cursor - MARK=$_ntr_mark if [[ -n $_ntr_save ]]; then - eval "$_ntr_save=(${(qq)_ntr_predisplay} ${(qq)_ntr_postdisplay} -${(qq)_ntr_lbuffer} ${(qq)_ntr_rbuffer})" || return 1 + builtin typeset -ga $_ntr_save + set -A $_ntr_save "${_ntr_predisplay}" "${_ntr_postdisplay}" \ + "${_ntr_lbuffer}" "${_ntr_rbuffer}" || return 1 fi + + BUFFER=${BUFFER[_ntr_start+1,_ntr_end-1]} + CURSOR=$_ntr_cursor + MARK=$_ntr_mark fi if [[ -z $_ntr_save && -z $_ntr_restore ]]; then zle recursive-edit _ntr_stat=$? - [[ -n $_ntr_lbuf_return ]] && eval "${_ntr_lbuf_return}=\${LBUFFER}" - [[ -n $_ntr_rbuf_return ]] && eval "${_ntr_rbuf_return}=\${RBUFFER}" + [[ -n $_ntr_lbuf_return ]] && + builtin typeset -g ${_ntr_lbuf_return}="${LBUFFER}" + [[ -n $_ntr_rbuf_return ]] && + builtin typeset -g ${_ntr_rbuf_return}="${RBUFFER}" fi if [[ -n $_ntr_restore || -z $_ntr_save ]]; then if [[ -n $_ntr_restore ]]; then - if ! eval "_ntr_predisplay=\${${_ntr_restore}[1]} -_ntr_postdisplay=\${${_ntr_restore}[2]} -_ntr_lbuffer=\${${_ntr_restore}[3]} -_ntr_rbuffer=\${${_ntr_restore}[4]}"; then - zle -M Failed. + 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 + zle -M Failed. >&2 return 1 fi fi PREDISPLAY=$_ntr_predisplay POSTDISPLAY=$_ntr_postdisplay - LBUFFER="$_ntr_lbuffer$LBUFFER" - RBUFFER="$RBUFFER$_ntr_rbuffer" + LBUFFER="$_ntr_lbuffer$BUFFER" + RBUFFER="$_ntr_rbuffer" + MARK=${#_ntr_lbuffer} fi return $_ntr_stat ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PATCH: narrow-to-region (was Re: This widget implementation feels a bit clunky) 2015-07-18 23:42 ` PATCH: narrow-to-region (was Re: This widget implementation feels a bit clunky) Bart Schaefer @ 2015-07-19 3:55 ` Oliver Kiddle 2015-07-19 8:23 ` Bart Schaefer 0 siblings, 1 reply; 19+ messages in thread From: Oliver Kiddle @ 2015-07-19 3:55 UTC (permalink / raw) To: zsh workers > emulate -L zsh > setopt extendedglob These lines (not introduced by Bart's patch) have the effect of resetting lots of options when inside the recursive-edit. That affects the behaviour of some things such as completion. How much of emulate zsh is actually needed by narrow-to-region itself? Can we get by with just setopt localoptions and a few extras? Or use an anonymous function around everything but recursive-edit? It'd also be good for the function to use the new undo limit features. Oliver ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PATCH: narrow-to-region (was Re: This widget implementation feels a bit clunky) 2015-07-19 3:55 ` Oliver Kiddle @ 2015-07-19 8:23 ` Bart Schaefer 2015-07-20 9:19 ` Oliver Kiddle 0 siblings, 1 reply; 19+ messages in thread From: Bart Schaefer @ 2015-07-19 8:23 UTC (permalink / raw) To: zsh workers On Jul 19, 5:55am, Oliver Kiddle wrote: } } How much of emulate zsh is actually needed by narrow-to-region itself? It uses a bunch of unquoted variable references without braces so it clearly expects no-shwordsplit and no-ksharrays. I don't see any need for extendedglob, on another quick read-through. } It'd also be good for the function to use the new undo limit features. I was going to send a follow-up message about undo-ing and whether a call to split-undo is needed before (and/or after) the recursive-edit. Related, should -S / -R implicitly set undo points? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PATCH: narrow-to-region (was Re: This widget implementation feels a bit clunky) 2015-07-19 8:23 ` Bart Schaefer @ 2015-07-20 9:19 ` Oliver Kiddle 2015-07-20 16:29 ` Bart Schaefer 2015-07-23 5:41 ` Undo and narrow-to-region (was Re: PATCH: narrow-to-region (was ...)) Bart Schaefer 0 siblings, 2 replies; 19+ messages in thread From: Oliver Kiddle @ 2015-07-20 9:19 UTC (permalink / raw) To: zsh workers Bart wrote: > } It'd also be good for the function to use the new undo limit features. > > I was going to send a follow-up message about undo-ing and whether a call > to split-undo is needed before (and/or after) the recursive-edit. It is needed immediately after you've setup PREDISPLAY and emptied BUFFER. I would have been fairly sure of that answer but my own attempts to make it work had failed so I went back to see again first. I now realise that the problem I had is elsewhere. I've attached a very rudimentary patch to narrow-to-region. My main use of narrow-to-region is to retrieve history lines into the middle of the current line. And it is just this situation that is breaking it. When doing: zle undo $_ntr_changeno it is falling foul of the first line of unapplychange which is: if(ch->hist != histline) { and not undoing enough changes. This is a consequence of the fix in 10328. If I comment out the two lines of undo() which do else break; it appears to work fine and the problem that 10328 targetted doesn't seem to come back. But it is hard to know how that was failing at the time. > Related, should -S / -R implicitly set undo points? I've never used those two options and am not sure I quite understand what they're for. At least not without digging in the archives. With the patch below, it is fairly obviously broken for them because _ntr_changeno is set in a different block from the one which does zle undo to revert to it. Oliver diff --git a/Functions/Zle/narrow-to-region b/Functions/Zle/narrow-to-region index 293f89b..ae4addd 100644 --- a/Functions/Zle/narrow-to-region +++ b/Functions/Zle/narrow-to-region @@ -29,8 +29,9 @@ emulate -L zsh setopt extendedglob -local _ntr_lbuf_return _ntr_rbuf_return +local _ntr_new_lbuf _ntr_newrbuf _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 @@ -98,6 +99,7 @@ if [[ -n $_ntr_save || -z $_ntr_restore ]]; then then _ntr_posttext=$_ntr_rbuffer fi + _ntr_changeno=$UNDO_CHANGE_NO PREDISPLAY="$_ntr_predisplay$_ntr_pretext" POSTDISPLAY="$_ntr_posttext$_ntr_postdisplay" @@ -110,16 +112,25 @@ if [[ -n $_ntr_save || -z $_ntr_restore ]]; then BUFFER=${BUFFER[_ntr_start+1,_ntr_end-1]} CURSOR=$_ntr_cursor MARK=$_ntr_mark + zle split-undo fi if [[ -z $_ntr_save && -z $_ntr_restore ]]; then - zle recursive-edit - _ntr_stat=$? - - [[ -n $_ntr_lbuf_return ]] && - builtin typeset -g ${_ntr_lbuf_return}="${LBUFFER}" - [[ -n $_ntr_rbuf_return ]] && - builtin typeset -g ${_ntr_rbuf_return}="${RBUFFER}" + { + UNDO_LIMIT_NO=$UNDO_CHANGE_NO + zle recursive-edit + _ntr_stat=$? + _ntr_newlbuf="$LBUFFER" + _ntr_newrbuf="$RBUFFER" + + [[ -n $_ntr_lbuf_return ]] && + builtin typeset -g ${_ntr_lbuf_return}="${LBUFFER}" + [[ -n $_ntr_rbuf_return ]] && + builtin typeset -g ${_ntr_rbuf_return}="${RBUFFER}" + } always { + zle undo $_ntr_changeno + UNDO_LIMIT_NO=_ntr_savelim + } fi if [[ -n $_ntr_restore || -z $_ntr_save ]]; then @@ -135,8 +146,8 @@ if [[ -n $_ntr_restore || -z $_ntr_save ]]; then PREDISPLAY=$_ntr_predisplay POSTDISPLAY=$_ntr_postdisplay - LBUFFER="$_ntr_lbuffer$BUFFER" - RBUFFER="$_ntr_rbuffer" + LBUFFER="$_ntr_lbuffer$_ntr_newlbuf" + RBUFFER="$_ntr_newrbuf$_ntr_rbuffer" MARK=${#_ntr_lbuffer} fi ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PATCH: narrow-to-region (was Re: This widget implementation feels a bit clunky) 2015-07-20 9:19 ` Oliver Kiddle @ 2015-07-20 16:29 ` Bart Schaefer 2015-07-23 5:41 ` Undo and narrow-to-region (was Re: PATCH: narrow-to-region (was ...)) Bart Schaefer 1 sibling, 0 replies; 19+ messages in thread From: Bart Schaefer @ 2015-07-20 16:29 UTC (permalink / raw) To: zsh workers On Jul 20, 11:19am, Oliver Kiddle wrote: } Subject: Re: PATCH: narrow-to-region (was Re: This widget implementation f } } Bart wrote: } > } It'd also be good for the function to use the new undo limit features. } > } > I was going to send a follow-up message about undo-ing and whether a call } > to split-undo is needed before (and/or after) the recursive-edit. } } It is needed immediately after you've setup PREDISPLAY and emptied } BUFFER. The sourceforge outage is getting a bit annoying with respect to passing these patches back and forth. } > Related, should -S / -R implicitly set undo points? } } I've never used those two options and am not sure I quite understand } what they're for. At least not without digging in the archives. With the } patch below, it is fairly obviously broken for them because } _ntr_changeno is set in a different block from the one which does zle } undo to revert to it. The -S and -R options are basically a hack to because there's no good way to do a callback mechansim. What you'd like is to replace recursive-edit with a call back into the invoking scope, but you can't do that without real closures or at least real namerefs, so "narrow-to-region -S" does everything up to recursive-edit and then saves state, followed by -R which restores state and picks up where -S left off. If you introduce new variables _ntr_savelim and _ntr_changeno those are going to have to be added to the state save/restore, or else they have to be localized to the part where recursive-edit is called and NOT the parts where state is saved/restored. Also this -- } PREDISPLAY=$_ntr_predisplay } POSTDISPLAY=$_ntr_postdisplay } - LBUFFER="$_ntr_lbuffer$BUFFER" } - RBUFFER="$_ntr_rbuffer" } + LBUFFER="$_ntr_lbuffer$_ntr_newlbuf" } + RBUFFER="$_ntr_newrbuf$_ntr_rbuffer" } MARK=${#_ntr_lbuffer} -- breaks MARK and CURSOR again, you need LBUFFER="$_ntr_lbuffer$_ntr_newlbuf$_ntr_newrbuf" RBUFFER="$_ntr_rbuffer" to get CURSOR in the right place. Which means you may as well not save the left and right buffers separately to begin with. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Undo and narrow-to-region (was Re: PATCH: narrow-to-region (was ...)) 2015-07-20 9:19 ` Oliver Kiddle 2015-07-20 16:29 ` Bart Schaefer @ 2015-07-23 5:41 ` Bart Schaefer 2015-07-27 15:44 ` Oliver Kiddle 1 sibling, 1 reply; 19+ messages in thread From: Bart Schaefer @ 2015-07-23 5:41 UTC (permalink / raw) To: zsh workers On Jul 20, 11:19am, Oliver Kiddle wrote: } Subject: Re: PATCH: narrow-to-region (was Re: This widget implementation f } } My main use of narrow-to-region is to retrieve history lines into the } middle of the current line. And it is just this situation that is } breaking it. When doing: } zle undo $_ntr_changeno } it is falling foul of the first line of unapplychange which is: } if(ch->hist != histline) { } and not undoing enough changes. } } This is a consequence of the fix in 10328. If I comment out the two } lines of undo() which do else break; it appears to work fine and the } problem that 10328 targetted doesn't seem to come back. But it is hard } to know how that was failing at the time. So you mean that part of undo becomes if (unapplychange(prev)) curchange = prev; } while (last_change >= (zlong)0 || (curchange->flags & CH_PREV)); ?? What must be happening is that unapplychange() alters the history line, returns zero, and then is called again with exactly the same struct, which now succeeds on the adjusted history line. Pre-10328, the call to unapplychange() would fail but curchange was moved to ->prev either way, skipping over the change that failed. So the "else break;" looks like paranoia to prevent an infinite loop in the event that unapplychange() fails every time. Which could happen if quietgethist() is a no-op? Perhaps when a line has fallen out of the history because of HISTSIZE? Anyway modulo infinite-loop-prevention, it looks to me as if removing that else-clause is the right thing to do, and similarly in redo() a bit further along. I'll let Oliver do the patch if we all agree. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Undo and narrow-to-region (was Re: PATCH: narrow-to-region (was ...)) 2015-07-23 5:41 ` Undo and narrow-to-region (was Re: PATCH: narrow-to-region (was ...)) Bart Schaefer @ 2015-07-27 15:44 ` Oliver Kiddle 2015-08-12 17:00 ` PATCH: Re: Undo and narrow-to-region (was ...) Oliver Kiddle 0 siblings, 1 reply; 19+ messages in thread From: Oliver Kiddle @ 2015-07-27 15:44 UTC (permalink / raw) To: zsh workers On 22 Jul, Bart wrote: > So you mean that part of undo becomes > > if (unapplychange(prev)) > curchange = prev; > } while (last_change >= (zlong)0 || (curchange->flags & CH_PREV)); > > ?? > > What must be happening is that unapplychange() alters the history line, > returns zero, and then is called again with exactly the same struct, Yes. Changes to the history number are not collected up as change events in nextchanges. This means that several history line changes are undone in a single go. It also has the, perhaps less desirable, effect that you can't use redo to revert a change to the history line. Is that worth fixing? > So the "else break;" looks like paranoia to prevent an infinite loop in > the event that unapplychange() fails every time. Which could happen if > quietgethist() is a no-op? Perhaps when a line has fallen out of the > history because of HISTSIZE? I tried to find a way to induce such an infinite loop and couldn't. Even so, how about the following patch which checks histline had changed to detect a failure to set the history line. > Anyway modulo infinite-loop-prevention, it looks to me as if removing > that else-clause is the right thing to do, and similarly in redo() a > bit further along. I'll let Oliver do the patch if we all agree. Not sure it's necessary for redo because there's no option to redo more than one change. Oliver diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c index 8b55403..d512c7a 100644 --- a/Src/Zle/zle_utils.c +++ b/Src/Zle/zle_utils.c @@ -1590,8 +1590,8 @@ undo(char **args) return 1; if (unapplychange(prev)) curchange = prev; - else - break; + else if (histline != prev->hist) + break; /* avoid infinite loop if history line unchanged */ } while (last_change >= (zlong)0 || (curchange->flags & CH_PREV)); setlastline(); return 0; ^ permalink raw reply [flat|nested] 19+ messages in thread
* PATCH: Re: Undo and narrow-to-region (was ...) 2015-07-27 15:44 ` Oliver Kiddle @ 2015-08-12 17:00 ` Oliver Kiddle 2015-08-17 21:15 ` Daniel Hahler 0 siblings, 1 reply; 19+ messages in thread From: Oliver Kiddle @ 2015-08-12 17:00 UTC (permalink / raw) To: zsh workers 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; } /**/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PATCH: Re: Undo and narrow-to-region (was ...) 2015-08-12 17:00 ` PATCH: Re: Undo and narrow-to-region (was ...) Oliver Kiddle @ 2015-08-17 21:15 ` Daniel Hahler 2015-08-17 21:58 ` Bart Schaefer 2015-08-18 7:05 ` Bart Schaefer 0 siblings, 2 replies; 19+ messages in thread From: Daniel Hahler @ 2015-08-17 21:15 UTC (permalink / raw) To: Oliver Kiddle; +Cc: Zsh Hackers' List, Christian Neukirchen -----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----- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PATCH: Re: Undo and narrow-to-region (was ...) 2015-08-17 21:15 ` Daniel Hahler @ 2015-08-17 21:58 ` Bart Schaefer 2015-08-18 3:43 ` Bart Schaefer 2015-08-18 7:05 ` Bart Schaefer 1 sibling, 1 reply; 19+ messages in thread From: Bart Schaefer @ 2015-08-17 21:58 UTC (permalink / raw) To: Zsh Hackers' List; +Cc: Daniel Hahler On Aug 17, 11:15pm, Daniel Hahler wrote: } Subject: Re: PATCH: Re: Undo and narrow-to-region (was ...) } } 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): Oliver may not be able to get to this for a while. Is it possible for you to try reverting just the change to the narrow-to-region shell code and see if that fixes things, or if both the shell and C code changes need to be reverted? There's been difficulty before with the save/restore state feature of narrow-to-region. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PATCH: Re: Undo and narrow-to-region (was ...) 2015-08-17 21:58 ` Bart Schaefer @ 2015-08-18 3:43 ` Bart Schaefer 0 siblings, 0 replies; 19+ messages in thread From: Bart Schaefer @ 2015-08-18 3:43 UTC (permalink / raw) To: Zsh Hackers' List On Aug 17, 2:58pm, I wrote: } } Oliver may not be able to get to this for a while. Is it possible for } you to try reverting just the change to the narrow-to-region shell code } and see if that fixes things, or if both the shell and C code changes } need to be reverted? I was able to reproduce the broken behavior with 832130c and then back that out to determine that Daniel's example works correctly with the previous version of narrow-to-region. Oliver said: > This is still not perfect with history changes. In fact it's actually horribly broken with respect to history changes, at least with -R / -S, as far as I can tell ... In Daniel's example, we start with BUFFER='', record $UNDO_CHANGE_NO, wander about in the history a bit, then do "zle undo $_ntr_changeno" with the expectation that will return us to the previous state, that is, an empty BUFFER. But it doesn't. It mostly works as long as BUFFER isn't empty when $UNDO_CHANGE_NO is saved. This goes back to Oliver's grumbling that undo decides what to do based on whether the text on the line matches the text at the undo point, In this case there is no text at the undo point, so anything will match and nothing gets undone. The following brute-forces the history back to the same line where the undo point was set before performing the undo. This works for Daniel's example but I have no idea what it will do for other cases. It can't make things any worse, in theory. diff --git a/Functions/Zle/narrow-to-region b/Functions/Zle/narrow-to-region index 0ef28a8..261d821 100644 --- a/Functions/Zle/narrow-to-region +++ b/Functions/Zle/narrow-to-region @@ -32,7 +32,7 @@ setopt localoptions noshwordsplit noksharrays 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_savelim=UNDO_LIMIT_NO _ntr_changeno _ntr_histno=HISTNO integer _ntr_start _ntr_end _ntr_swap _ntr_cursor=$CURSOR _ntr_mark=$MARK integer _ntr_stat @@ -108,7 +108,7 @@ if [[ -n $_ntr_save || -z $_ntr_restore ]]; then builtin typeset -ga $_ntr_save set -A $_ntr_save "${_ntr_predisplay}" "${_ntr_postdisplay}" \ "${_ntr_savelim}" "${_ntr_changeno}" \ - "${_ntr_start}" "${_ntr_end}" || return 1 + "${_ntr_start}" "${_ntr_end}" "${_ntr_histno}" || return 1 fi BUFFER=${BUFFER[_ntr_start+1,_ntr_end]} @@ -135,13 +135,15 @@ if [[ -n $_ntr_restore || -z $_ntr_save ]]; 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 + _ntr_end="${${(@P)_ntr_restore}[6]}" + _ntr_histno="${${(@P)_ntr_restore}[7]}" }; then zle -M Failed. >&2 return 1 fi fi _ntr_newbuf="$BUFFER" + HISTNO=_ntr_histno zle undo $_ntr_changeno PREDISPLAY=$_ntr_predisplay POSTDISPLAY=$_ntr_postdisplay -- Barton E. Schaefer ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PATCH: Re: Undo and narrow-to-region (was ...) 2015-08-17 21:15 ` Daniel Hahler 2015-08-17 21:58 ` Bart Schaefer @ 2015-08-18 7:05 ` Bart Schaefer 1 sibling, 0 replies; 19+ messages in thread From: Bart Schaefer @ 2015-08-18 7:05 UTC (permalink / raw) To: Zsh Hackers' List; +Cc: Daniel Hahler On Aug 17, 11:15pm, Daniel Hahler wrote: } } 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 } } Incidentally, the MARK=CURSOR "magick" doesn't seem to be needed any more with the most recent revision of narrow-to-region. However, if you remove that, then the retrieved history will *replace* whatever text starts out in the region between MARK and CURSOR. (This is consistent with what bracketed-paste does, if that matters.) -- Barton E. Schaefer ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-08-18 7:05 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-06-20 9:16 This widget implementation feels a bit clunky (edit-quoted-word) Mikael Magnusson 2015-06-20 17:06 ` Peter Stephenson 2015-06-21 7:09 ` Mikael Magnusson 2015-06-21 17:17 ` Peter Stephenson 2015-06-22 2:26 ` Mikael Magnusson 2015-06-22 0:18 ` PATCH: Document narrow-to-region -l and -r Mikael Magnusson 2015-06-20 17:21 ` This widget implementation feels a bit clunky (edit-quoted-word) Bart Schaefer 2015-07-18 23:42 ` PATCH: narrow-to-region (was Re: This widget implementation feels a bit clunky) Bart Schaefer 2015-07-19 3:55 ` Oliver Kiddle 2015-07-19 8:23 ` Bart Schaefer 2015-07-20 9:19 ` Oliver Kiddle 2015-07-20 16:29 ` Bart Schaefer 2015-07-23 5:41 ` Undo and narrow-to-region (was Re: PATCH: narrow-to-region (was ...)) Bart Schaefer 2015-07-27 15:44 ` Oliver Kiddle 2015-08-12 17:00 ` PATCH: Re: Undo and narrow-to-region (was ...) Oliver Kiddle 2015-08-17 21:15 ` Daniel Hahler 2015-08-17 21:58 ` Bart Schaefer 2015-08-18 3:43 ` Bart Schaefer 2015-08-18 7:05 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).