* Regression in bracketed-paste-magic (not in 5.2, but affects 5.3's test releases)
@ 2016-12-07 2:57 Zhiming Wang
2016-12-07 3:25 ` Bart Schaefer
0 siblings, 1 reply; 6+ messages in thread
From: Zhiming Wang @ 2016-12-07 2:57 UTC (permalink / raw)
To: zsh-workers
Commit 20948d0 (38579: Functions/Zle/bracketed-paste-magic: simplify saving and
restoring of state) introduced a regression that results in the ZLE buffer
being repeated when pasting under certain conditions. It was introduced after
5.2 and currently affects 5.2-test-1, 5.2-test-2 and master.
Here's a simple reproducer:
1. Set up environment (.zshrc and history) and open a new session with that
environment:
cd "$(mktemp -d /tmp/XXXXXXX)"
cat >.zshrc <<'EOF'
HISTFILE=$HOME/history
autoload -Uz bracketed-paste-magic
zle -N bracketed-paste bracketed-paste-magic
EOF
cat >history <<'EOF'
echo 1
echo 2
EOF
HOME=$PWD zsh
2. Press the following sequence of keys (no space): ^R echo ^R ^E
(history-incremental-search-backward, "echo",
history-incremental-search-backward, end-of-line), then paste something,
say, "zsh".
At this point, we should expect
echo 1zsh
in the ZLE buffer. Instead, we see
echo 1zshecho 1
note the buffer prior to pasting being repeated at the end.
Here's a terminal recording that demonstrates the wrong behavior in action:
https://asciinema.org/a/95254.
Best,
Zhiming
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Regression in bracketed-paste-magic (not in 5.2, but affects 5.3's test releases)
2016-12-07 2:57 Regression in bracketed-paste-magic (not in 5.2, but affects 5.3's test releases) Zhiming Wang
@ 2016-12-07 3:25 ` Bart Schaefer
2016-12-07 4:02 ` Zhiming Wang
2016-12-07 12:39 ` Oliver Kiddle
0 siblings, 2 replies; 6+ messages in thread
From: Bart Schaefer @ 2016-12-07 3:25 UTC (permalink / raw)
To: Zhiming Wang; +Cc: Zsh hackers list
[-- Attachment #1: Type: text/plain, Size: 908 bytes --]
On Dec 6, 2016 6:58 PM, "Zhiming Wang" <zmwangx@gmail.com> wrote:
Commit 20948d0 (38579: Functions/Zle/bracketed-paste-magic: simplify saving
and
restoring of state) introduced a regression that results in the ZLE buffer
being repeated when pasting under certain conditions.
[...]
Here's a simple reproducer:
2. Press the following sequence of keys (no space): ^R echo ^R ^E
(history-incremental-search-backward, "echo",
history-incremental-search-backward, end-of-line), then paste something,
say, "zsh".
This probably indicates a problem with undo-point handling around history
search, which has been a sticky wicket in the past. The patch you cite
replaced b-p-m's internal handling of $BUFFER with reliance on the undo
mechanism.
For the 5.3 release the easiest approach is likely to be to back out that
change and re-apply it after the release when it can be more thoroughly
debugged.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Regression in bracketed-paste-magic (not in 5.2, but affects 5.3's test releases)
2016-12-07 3:25 ` Bart Schaefer
@ 2016-12-07 4:02 ` Zhiming Wang
2016-12-07 12:39 ` Oliver Kiddle
1 sibling, 0 replies; 6+ messages in thread
From: Zhiming Wang @ 2016-12-07 4:02 UTC (permalink / raw)
To: Bart Schaefer; +Cc: zsh-workers
On Dec 6, 2016, at 10:25 PM, Bart Schaefer <schaefer@brasslantern.com> wrote:
> For the 5.3 release the easiest approach is likely to be to back out that
> change and re-apply it after the release when it can be more thoroughly
> debugged.
Sounds good. I have verified that a revert restores sanity, for now.
From c1e93b892d9e444d751a073baa329c25cd996471 Mon Sep 17 00:00:00 2001
From: Zhiming Wang <zmwangx@gmail.com>
Date: Tue, 6 Dec 2016 23:01:33 -0500
Subject: [PATCH] Revert "38579: simplify saving and restoring of state"
This reverts commit 20948d088994dc7b26a26b94926432985fa6863e.
---
Functions/Zle/bracketed-paste-magic | 44 +++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/Functions/Zle/bracketed-paste-magic b/Functions/Zle/bracketed-paste-magic
index fb584d595..c46f741d5 100644
--- a/Functions/Zle/bracketed-paste-magic
+++ b/Functions/Zle/bracketed-paste-magic
@@ -145,26 +145,27 @@ bracketed-paste-magic() {
done
fi
+ # Save context, create a clean slate for the paste
+ integer bpm_mark=$MARK bpm_cursor=$CURSOR bpm_region=$REGION_ACTIVE
+ integer bpm_numeric=${NUMERIC:-1}
+ local bpm_buffer=$BUFFER
+ fc -p -a /dev/null 0 0
+ BUFFER=
+
zstyle -a :bracketed-paste-magic inactive-keys bpm_inactive
if zstyle -s :bracketed-paste-magic active-widgets bpm_active '|'; then
- # Save context, create a clean slate for the paste
- integer bpm_mark=$MARK bpm_region=$REGION_ACTIVE
- integer bpm_numeric=${NUMERIC:-1}
- integer bpm_limit=$UNDO_LIMIT_NO bpm_undo=$UNDO_CHANGE_NO
- BUFFER=
- CURSOR=1
- zle .split-undo
- UNDO_LIMIT_NO=$UNDO_CHANGE_NO
- fc -p -a /dev/null 0 0
+ # There are active widgets. Reprocess $PASTED as keystrokes.
+ NUMERIC=1
+ zle -U - $PASTED
+
if [[ $bmp_keymap = vicmd ]]; then
zle -K viins
fi
- # There are active widgets. Reprocess $PASTED as keystrokes.
- NUMERIC=1
- zle -U - "$PASTED"
-
# Just in case there are active undo widgets
+ zle .split-undo
+ integer bpm_limit=$UNDO_LIMIT_NO bpm_undo=$UNDO_CHANGE_NO
+ UNDO_LIMIT_NO=$UNDO_CHANGE_NO
while [[ -n $PASTED ]] && zle .read-command; do
PASTED=${PASTED#$KEYS}
@@ -182,16 +183,21 @@ bracketed-paste-magic() {
done
PASTED=$BUFFER
- # Restore state
- zle -K $bpm_keymap
- fc -P
- MARK=$bpm_mark
- REGION_ACTIVE=$bpm_region
- NUMERIC=$bpm_numeric
+ # Reset the undo state
zle .undo $bpm_undo
UNDO_LIMIT_NO=$bpm_limit
+
+ zle -K $bpm_keymap
fi
+ # Restore state
+ BUFFER=$bpm_buffer
+ MARK=$bpm_mark
+ CURSOR=$bpm_cursor
+ REGION_ACTIVE=$bpm_region
+ NUMERIC=$bpm_numeric
+ fc -P
+
# PASTED has been updated, run the paste-finish functions
if zstyle -a :bracketed-paste-magic paste-finish bpm_hooks; then
for bpm_func in $bpm_hooks; do
--
2.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Regression in bracketed-paste-magic (not in 5.2, but affects 5.3's test releases)
2016-12-07 3:25 ` Bart Schaefer
2016-12-07 4:02 ` Zhiming Wang
@ 2016-12-07 12:39 ` Oliver Kiddle
2016-12-07 18:23 ` Zhiming Wang
2016-12-24 8:28 ` Bart Schaefer
1 sibling, 2 replies; 6+ messages in thread
From: Oliver Kiddle @ 2016-12-07 12:39 UTC (permalink / raw)
To: Zhiming Wang, Zsh hackers list
Bart wrote:
> This probably indicates a problem with undo-point handling around history
> search, which has been a sticky wicket in the past. The patch you cite
> replaced b-p-m's internal handling of $BUFFER with reliance on the undo
> mechanism.
In this particular case, the problem is that fc -p doesn't result
in a call to zle's remember_edits(). remember_edits() updates the
table of history entries for the current line. The edit associated
with doing BUFFER= CURSOR=1 is thus lost. So when it does the
undo, the line is first restored when the history line is reselected
and then the contents are duplicated when the effect of clearing
$BUFFER is undone.
Unfortunately, the ZLE_CMD_SET_HIST_LINE callback happens after this
history line is updated so we can't call remember_edits() from there.
We could simply add another similar hook and call it earlier.
What I would suggest for 5.3 is the patch below. fc -p doesn't
trigger handleundo() either so by moving split-undo above the
clearing of the buffer, we ensure that the edit is missed by both
the undo and history. Referencing $UNDO_CHANGE_NO does force
handleundo() so the downside of this is that UNDO_LIMIT_NO will
allow an undo back to the state before clearing $BUFFER.
If you're wondering why I suggest this rather than adding a hook
to call remember_edits(), it is because, for a proper post-5.3
patch, fc -p wouldn't actually change the current history line. The
only thing that changes is the history number. If history internals
use the Histent to identify history entries then undo wouldn't see
fc -p/-P as a change of history line and we could kill the associated
bugs. I've got some unfinished work on this on a git branch if
someone's interested to play with it. But for now, calling fc -p
in a widget is going to get you undo oddities.
I don't use bracketed-paste-magic so would appreciate if someone
else could do some testing and make the call on what should go into
5.3.
Oliver
diff --git a/Functions/Zle/bracketed-paste-magic b/Functions/Zle/bracketed-paste-magic
index fb584d5..c3cc8b6 100644
--- a/Functions/Zle/bracketed-paste-magic
+++ b/Functions/Zle/bracketed-paste-magic
@@ -151,10 +151,10 @@ bracketed-paste-magic() {
integer bpm_mark=$MARK bpm_region=$REGION_ACTIVE
integer bpm_numeric=${NUMERIC:-1}
integer bpm_limit=$UNDO_LIMIT_NO bpm_undo=$UNDO_CHANGE_NO
- BUFFER=
- CURSOR=1
zle .split-undo
UNDO_LIMIT_NO=$UNDO_CHANGE_NO
+ BUFFER=
+ CURSOR=1
fc -p -a /dev/null 0 0
if [[ $bmp_keymap = vicmd ]]; then
zle -K viins
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Regression in bracketed-paste-magic (not in 5.2, but affects 5.3's test releases)
2016-12-07 12:39 ` Oliver Kiddle
@ 2016-12-07 18:23 ` Zhiming Wang
2016-12-24 8:28 ` Bart Schaefer
1 sibling, 0 replies; 6+ messages in thread
From: Zhiming Wang @ 2016-12-07 18:23 UTC (permalink / raw)
To: Oliver Kiddle; +Cc: Zsh hackers list
On Dec 7, 2016, at 7:39 AM, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> I don't use bracketed-paste-magic so would appreciate if someone
> else could do some testing ...
>
> diff --git a/Functions/Zle/bracketed-paste-magic b/Functions/Zle/bracketed-paste-magic
> index fb584d5..c3cc8b6 100644
> --- a/Functions/Zle/bracketed-paste-magic
> +++ b/Functions/Zle/bracketed-paste-magic
> @@ -151,10 +151,10 @@ bracketed-paste-magic() {
> integer bpm_mark=$MARK bpm_region=$REGION_ACTIVE
> integer bpm_numeric=${NUMERIC:-1}
> integer bpm_limit=$UNDO_LIMIT_NO bpm_undo=$UNDO_CHANGE_NO
> - BUFFER=
> - CURSOR=1
> zle .split-undo
> UNDO_LIMIT_NO=$UNDO_CHANGE_NO
> + BUFFER=
> + CURSOR=1
> fc -p -a /dev/null 0 0
> if [[ $bmp_keymap = vicmd ]]; then
> zle -K viins
I applied the patch and played around for a while; the issue appears to be
fixed.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Regression in bracketed-paste-magic (not in 5.2, but affects 5.3's test releases)
2016-12-07 12:39 ` Oliver Kiddle
2016-12-07 18:23 ` Zhiming Wang
@ 2016-12-24 8:28 ` Bart Schaefer
1 sibling, 0 replies; 6+ messages in thread
From: Bart Schaefer @ 2016-12-24 8:28 UTC (permalink / raw)
To: Zsh hackers list
On Dec 7, 1:39pm, Oliver Kiddle wrote:
}
} In this particular case, the problem is that fc -p doesn't result
} in a call to zle's remember_edits(). remember_edits() updates the
} table of history entries for the current line. The edit associated
} with doing BUFFER= CURSOR=1 is thus lost. So when it does the
} undo, the line is first restored when the history line is reselected
} and then the contents are duplicated when the effect of clearing
} $BUFFER is undone.
This reminds me of a question I had when 38579 was applied: What's
the point of assigning CURSOR=1 here? If BUFFER is empty, the only
valid value of CURSOR is 0 (isn't it?), and the assignment to BUFFER
resets CURSOR implicitly as far as I can tell.
} What I would suggest for 5.3 is the patch below. [...]
}
} If you're wondering why I suggest this rather than adding a hook
} to call remember_edits(), it is because, for a proper post-5.3
} patch, fc -p wouldn't actually change the current history line.
Now that 5.3.1 is out, I've re-commited bracketed-paste-magic to its
state after 38579 plus the patch in this thread (40118), in spite of
my question above. We can now begin fiddling with the proper post-5.3
fix to "fc -p".
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-24 8:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-07 2:57 Regression in bracketed-paste-magic (not in 5.2, but affects 5.3's test releases) Zhiming Wang
2016-12-07 3:25 ` Bart Schaefer
2016-12-07 4:02 ` Zhiming Wang
2016-12-07 12:39 ` Oliver Kiddle
2016-12-07 18:23 ` Zhiming Wang
2016-12-24 8:28 ` 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).