Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm
Precedence: bulk
In-reply-to: <160602002257.ZM14758@torch.brasslantern.com>
From: Oliver Kiddle <okiddle@yahoo.co.uk>
References: <CAHYJk3TPHXFx96mm-40hvEqnd__8v8mnebiZ1r+thxGorZz=qg@mail.gmail.com> <7795.1464138176@thecus.kiddle.eu> <160602002257.ZM14758@torch.brasslantern.com>
To: zsh workers <zsh-workers@zsh.org>
Subject: Re: Bug in undo/redo handling in conjunction with history-beginning-search-backward
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <22809.1464880333.1@thecus.kiddle.eu>
Date: Thu, 02 Jun 2016 17:12:13 +0200
Message-ID: <22810.1464880333@thecus.kiddle.eu>

Bart wrote:
> I don't think this is directly related to history changes, I can cause a
> crash on multiple undo with bracketed-paste-magic having not changed the
> history line at all.  See other thread.

Actually it looks like history lines are involved.
bracketed-paste-magic does fc -p which invalidates HISTNO.
After bracketed-paste-magic, dumping the undo structure using a gdb
macro shows the following (| denotes merged changes):

No.	Offset	Delete          Insert         Cursor Hist
 8  	0	                echo https://bu 0 61   803
 6  	0	echo                            5  0     0
 5 |	4	                                4  5   804
 4 |	3	                o               3  4   804
 3 |	2	                h               2  3   804
 2 |	1	                c               1  2   804
 1 |	0	                e               0  1   804

So one undo blanks the whole line; a second undo tries to move to history line
0 - which fails.

fc -p/-P are not part of zle so don't save histline. This also explains
why change 8 has a history line of 803 instead of 804. Use of fc -p
within zle appears to be unique to bracketed-paste-magic. What is the
reason for using it? Are you trying to block history widgets even where
they are listed in active-widgets?

So does anyone have any thoughts on how to handle this. The simplest is
for fc -p/-P to detect zle being active and print an error. Perhaps we
need some module callbacks for pushing and popping history. Zle would
hook the callback, save histline and somehow mark this in the undo
structure.

I attach a patch to the state saving and restoring in
bracketed-paste-magic. Don't expect this to fix anything. I did it to
make things easier to understand. Was there a reason for half the state
saving/restoring to be outside the if statement? Saving BUFFER and
CURSOR is fairly pointless if undo is going to restore it. It'd be good
if this could be tested by someone who does use bracketed-paste-magic,
however.

Oliver

diff --git a/Functions/Zle/bracketed-paste-magic b/Functions/Zle/bracketed-paste-magic
index cafe18e..00cdb92 100644
--- a/Functions/Zle/bracketed-paste-magic
+++ b/Functions/Zle/bracketed-paste-magic
@@ -145,27 +145,26 @@ 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
-        # There are active widgets.  Reprocess $PASTED as keystrokes.
-	NUMERIC=1
-	zle -U - $PASTED
-
+	# 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
 	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}
@@ -183,21 +182,16 @@ bracketed-paste-magic() {
 	done
 	PASTED=$BUFFER
 
-	# Reset the undo state
+	# Restore state
+	zle -K $bpm_keymap
+	fc -P
+	MARK=$bpm_mark
+	REGION_ACTIVE=$bpm_region
+	NUMERIC=$bpm_numeric
 	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