[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#16818: Acknowledgement (Undo in region after markers in undo history
From: |
Stefan |
Subject: |
bug#16818: Acknowledgement (Undo in region after markers in undo history relocated) |
Date: |
Mon, 24 Mar 2014 09:03:50 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) |
> Attached is the new patch to solve this, implementing option 2 and
> incorporating your feedback.
Looks good, thank you very much. Feel free to install into emacs-24,
but please see my comments below.
Stefan
> +2014-03-13 Barry O'Reilly <gundaetiapo@gmail.com>
> + * markers.texi (Moving Marker Positions): The 2014-03-02 doc
Missing empty line between the two.
> + (while (and (consp (car list))
> + (markerp (caar list))
(markerp (car-safe (car list)))
> + (and (eq (marker-buffer (caar list))
> + (current-buffer))
> + (integerp (marker-position (caar list)))
I think this `integerp' test is redundant (marker-position can only
return nil or an integer and if it returns nil, then marker-buffer also
returns nil).
> ;; (MARKER . OFFSET) means a marker MARKER was adjusted by OFFSET.
> (`(,(and marker (pred markerp)) . ,(and offset (pred integerp)))
> - (when (marker-buffer marker)
> - (set-marker marker
> - (- marker offset)
> - (marker-buffer marker))))
> + (warn "Encountered %S entry in undo list with no matching (TEXT .
> POS) entry"
> + next))
I think I'd add the warning without removing the `set-marker', just to
be conservative.
> + (when (and (consp undo-elt)
> + (stringp (car undo-elt))
(stringp (car-safe undo-elt))
> + (integerp (cdr undo-elt)))
> + (let ((list-i (cdr undo-list-copy)))
> + (while (markerp (caar list-i))
I think you need (markerp (car-safe (car list-i))) because (car list-i)
may be an integer.
Also, I'd try to avoid repeatedly calling (car list-i) in the body of
the `while' loop(s) by let-binding it. I'd probably do it via `pop', as
in
(while (...)
(let ((elem (pop undo-list)))
...))
> ((and (consp undo-elt) (markerp (car undo-elt)))
> - ;; This is a marker-adjustment element (MARKER . ADJUSTMENT).
> - ;; See if MARKER is inside the region.
> - (let ((alist-elt (assq (car undo-elt) undo-adjusted-markers)))
> - (unless alist-elt
> - (setq alist-elt (cons (car undo-elt)
> - (marker-position (car undo-elt))))
> - (setq undo-adjusted-markers
> - (cons alist-elt undo-adjusted-markers)))
> - (and (cdr alist-elt)
> - (>= (cdr alist-elt) start)
> - (<= (cdr alist-elt) end))))
> + ;; (MARKER . ADJUSTMENT)
> + nil)
It would also be more conservative to keep this unchanged, but I think
I agree with you here that we don't need to be *that* conservative.
> - record_delete (beg, make_buffer_string (beg, beg + length, 1));
> + record_delete (beg, make_buffer_string (beg, beg + length, 1), false);
Begs the question: why didn't (don't) we record marker adjustments here
(and other similar places)?
Stefan
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), (continued)
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Stefan Monnier, 2014/03/13
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Stefan Monnier, 2014/03/13
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Barry OReilly, 2014/03/13
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Barry OReilly, 2014/03/17
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Stefan, 2014/03/17
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Barry OReilly, 2014/03/19
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Stefan, 2014/03/19
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Barry OReilly, 2014/03/19
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Stefan, 2014/03/19
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Barry OReilly, 2014/03/23
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated),
Stefan <=
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Barry OReilly, 2014/03/24
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Stefan, 2014/03/24
- bug#16818: Acknowledgement (Undo in region after markers in undo history relocated), Barry OReilly, 2014/03/24