zsh-workers
 help / color / mirror / code / Atom feed
From: Oliver Kiddle <okiddle@yahoo.co.uk>
To: zsh workers <zsh-workers@zsh.org>
Subject: Re: Bug in undo/redo handling in conjunction with history-beginning-search-backward
Date: Thu, 02 Jun 2016 17:12:13 +0200	[thread overview]
Message-ID: <22810.1464880333@thecus.kiddle.eu> (raw)
In-Reply-To: <160602002257.ZM14758@torch.brasslantern.com>

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


  reply	other threads:[~2016-06-02 15:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-20  5:25 Mikael Magnusson
2016-05-25  1:02 ` Oliver Kiddle
2016-06-02  7:22   ` Bart Schaefer
2016-06-02 15:12     ` Oliver Kiddle [this message]
2016-06-02 22:09       ` Bart Schaefer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22810.1464880333@thecus.kiddle.eu \
    --to=okiddle@yahoo.co.uk \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).