bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#29118: 25.2.50; Undoing shell flush output results in weird state


From: Noam Postavsky
Subject: bug#29118: 25.2.50; Undoing shell flush output results in weird state
Date: Sun, 12 Nov 2017 13:30:22 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.90 (gnu/linux)

found 29118 24.4
quit

Allen Li <vianchielfaura@gmail.com> writes:

> 2. Changing the process-mark insertion type before undoing prevents the bug:
>
> (set-marker-insertion-type (process-mark (get-buffer-process
> (current-buffer))) t)

Ah, interesting.

> However, you need to set it back to nil after undoing or all user
> input will get inserted before process-mark.
>
> So one way to fix this would be to define a comint-undo that sets and
> resets the process-mark insertion type.
> That's probably not the most elegant solution.
>
> I think undo in general does not behave well with markers. undo will
> always rely on the insertion type of the markers,
> whereas the text to re-insert may well have been on either side of the marker.
>
> Perhaps undo should be improved to track which side of a marker
> deleted text was at.

Hmm, as far as I can tell, in this case the text was inserted on the
"normal" side of the marker, but then the marker is moved afterwards.
So I'm not sure if such tracking would help.

    (defun comint-output-filter (process string)
       ...
            ;; insert-before-markers is a bad thing. XXX
            ;; Luckily we don't have to use it any more, we use
            ;; window-point-insertion-type instead.
            (insert string)

            ;; Advance process-mark
            (set-marker (process-mark process) (point))


However, while looking at the undo code, I noticed that it checks for
valid marker adjustments by checking that the marker position is the
same as the POSITION of the deletion record.  However, for a negative
POSITION (indicating text was at the end of the deleted text) this
comparison will always be false (markers can't have negative positions).

  (defun primitive-undo (n list)
    ...          
            (`(,(and string (pred stringp)) . ,(and pos (pred integerp)))
                   ...
                   (and (eq (marker-buffer m) (current-buffer))
                        (= pos m) ; <<<<<<<<<<<< Always nil for negative `pos'
                        (push marker-adj valid-marker-adjustments))))

I don't see why the position of point should affect the adjustment of
other markers.  This seems to have been added by [1: 37ea8275f7] to fix
Bug#16818.  Before that, if I understand correctly, all marker
adjustments were applied without checking location at all.  And indeed,
this bug does not occur in Emacs 24.3.

So I would suggest the following patch, which does seem to actually fix
this bug.  It might break some other things though; I have not tested it
apart from the scenario in this bug.  I'm adding the participants of
Bug#16818 to Cc, in the hope they might have some more insight.

--- i/lisp/simple.el
+++ w/lisp/simple.el
@@ -2579,7 +2579,7 @@ primitive-undo
                (let* ((marker-adj (pop list))
                       (m (car marker-adj)))
                  (and (eq (marker-buffer m) (current-buffer))
-                      (= pos m)
+                      (= (abs pos) m)
                       (push marker-adj valid-marker-adjustments))))
              ;; Insert string and adjust point
              (if (< pos 0)

[1: 37ea8275f7]: 2014-03-24 22:47:39 -0400
  Undo in region after markers in undo history relocated
  
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=37ea8275f7faad1192ddaba9f4a0789580675e17





reply via email to

[Prev in Thread] Current Thread [Next in Thread]