fix for bug#29935 copyright-update inserts year at random places

fix for bug#29935 copyright-update inserts year at random places

[Stephen Leake]

I filed bug#29935 because copyright-update sometimes inserts the new
year at random places (happy new year! :).

This was discussed last April, but no bug report was filed, and both
master and emacs-26 still have the bug: see
https://lists.gnu.org/archive/html/emacs-devel/2017-10/msg00848.html

I improved the fix posted there; see below.

Ok to commit to emacs-26?

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=29935
diff --git a/lisp/emacs-lisp/copyright.el b/lisp/emacs-lisp/copyright.el
index 69c5ebd45d..dcbee62af6 100644
--- a/lisp/emacs-lisp/copyright.el
+++ b/lisp/emacs-lisp/copyright.el
@@ -181,19 +181,22 @@ copyright-update-year
   ;; This uses the match-data from copyright-find-copyright/end.
   (goto-char (match-end 1))
   (copyright-find-end)
-  (setq copyright-current-year (format-time-string "%Y"))
-  (unless (string= (buffer-substring (- (match-end 3) 2) (match-end 3))
-		   (substring copyright-current-year -2))
-    (if (or noquery
-	    (save-window-excursion
-	      (switch-to-buffer (current-buffer))
-	      ;; Fixes some point-moving oddness (bug#2209).
-	      (save-excursion
-		(y-or-n-p (if replace
-			      (concat "Replace copyright year(s) by "
-				      copyright-current-year "? ")
-			    (concat "Add " copyright-current-year
-				    " to copyright? "))))))
+  (let ((copyright-end (point)))
+    (setq copyright-current-year (format-time-string "%Y"))
+    (unless (string= (buffer-substring (- (match-end 3) 2) (match-end 3))
+		     (substring copyright-current-year -2))
+      (if (or noquery
+	      (save-window-excursion
+	        ;; Fixes some point-moving oddness (bug#2209, bug#29935).
+	        (save-excursion
+	          (switch-to-buffer (current-buffer))
+                  ;; Ensure the copyright line is displayed.
+                  (goto-char copyright-end)
+		  (y-or-n-p (if replace
+			        (concat "Replace copyright year(s) by "
+				        copyright-current-year "? ")
+			      (concat "Add " copyright-current-year
+				      " to copyright? "))))))
 	(if replace
 	    (replace-match copyright-current-year t t nil 3)
 	  (let ((size (save-excursion (skip-chars-backward "0-9"))))
@@ -218,7 +221,8 @@ copyright-update-year
 	      (if (eq (char-after (+ (point) size -3)) ?')
 		  (insert ?')))
 	    ;; Finally insert the new year.
-	    (insert (substring copyright-current-year size)))))))
+	    (insert (substring copyright-current-year size))))
+        ))))
 
 ;;;###autoload
 (defun copyright-update (&optional arg interactivep)

-- 
-- Stephe

Re: fix for bug#29935 copyright-update inserts year at random places

[Lele Gaifax]

Thank you Stephen!

I didn't try your version (yet), but it seems almost equivalent to what I'm
using since April (the only difference being your explicit `(goto-char
copyright-end)' to "ensure the copyright line is displayed").

ciao, lele.
-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.

Re: fix for bug#29935 copyright-update inserts year at random places

[Eli Zaretskii]

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Date: Mon, 01 Jan 2018 17:29:27 -0600
> 
> Ok to commit to emacs-26?

How important is it to have this in Emacs 26?

> +  (let ((copyright-end (point)))
> +    (setq copyright-current-year (format-time-string "%Y"))
> +    (unless (string= (buffer-substring (- (match-end 3) 2) (match-end 3))
> +		     (substring copyright-current-year -2))
> +      (if (or noquery
> +	      (save-window-excursion
> +	        ;; Fixes some point-moving oddness (bug#2209, bug#29935).
> +	        (save-excursion
> +	          (switch-to-buffer (current-buffer))
> +                  ;; Ensure the copyright line is displayed.
> +                  (goto-char copyright-end)
> +		  (y-or-n-p (if replace
> +			        (concat "Replace copyright year(s) by "
> +				        copyright-current-year "? ")
> +			      (concat "Add " copyright-current-year
> +				      " to copyright? "))))))

Why does this need to use save-window-excursion?  AFAIU, that's the
root cause of the problem, so did you try replacing it with equivalent
code that uses other facilities for the same purpose?

Thanks.

Re: fix for bug#29935 copyright-update inserts year at random places

[Stephen Leake]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stephen Leake <stephen_leake@stephe-leake.org>
>> Date: Mon, 01 Jan 2018 17:29:27 -0600
>>
>> Ok to commit to emacs-26?
>
> How important is it to have this in Emacs 26?
>
>> +  (let ((copyright-end (point)))
>> +    (setq copyright-current-year (format-time-string "%Y"))
>> +    (unless (string= (buffer-substring (- (match-end 3) 2) (match-end 3))
>> +		     (substring copyright-current-year -2))
>> +      (if (or noquery
>> +	      (save-window-excursion
>> +	        ;; Fixes some point-moving oddness (bug#2209, bug#29935).
>> +	        (save-excursion
>> +	          (switch-to-buffer (current-buffer))
>> +                  ;; Ensure the copyright line is displayed.
>> +                  (goto-char copyright-end)
>> +		  (y-or-n-p (if replace
>> +			        (concat "Replace copyright year(s) by "
>> +				        copyright-current-year "? ")
>> +			      (concat "Add " copyright-current-year
>> +				      " to copyright? "))))))
>
> Why does this need to use save-window-excursion?

'switch-to-buffer' can pop up a new frame, or use a different window,
depending on various settings, so save-window-excursion is needed. 

'(goto-char copyright-end)' is needed because in general, a buffer does
not have a well-defined 'point'; is has a point in each window it is
displayed in, and recent Emacsen preserve that point when the buffer is
swapped in and out of that window (which is a nice feature).
'copyright-find-end' changes point in the buffer, but that's not used if
it's not displayed in the current window.

You suggest "using other facilities", but isn't this precisely what
save-window-excursion and save-excursion are for?

Here's a new patch, with improved comments:

--- a/lisp/emacs-lisp/copyright.el
+++ b/lisp/emacs-lisp/copyright.el
@@ -181,44 +181,54 @@ copyright-update-year
   ;; This uses the match-data from copyright-find-copyright/end.
   (goto-char (match-end 1))
   (copyright-find-end)
-  (setq copyright-current-year (format-time-string "%Y"))
-  (unless (string= (buffer-substring (- (match-end 3) 2) (match-end 3))
-		   (substring copyright-current-year -2))
-    (if (or noquery
-	    (save-window-excursion
-	      (switch-to-buffer (current-buffer))
-	      ;; Fixes some point-moving oddness (bug#2209).
-	      (save-excursion
-		(y-or-n-p (if replace
-			      (concat "Replace copyright year(s) by "
-				      copyright-current-year "? ")
-			    (concat "Add " copyright-current-year
-				    " to copyright? "))))))
-	(if replace
-	    (replace-match copyright-current-year t t nil 3)
-	  (let ((size (save-excursion (skip-chars-backward "0-9"))))
-	    (if (and (eq (% (- (string-to-number copyright-current-year)
-			       (string-to-number (buffer-substring
-						  (+ (point) size)
-						  (point))))
-			    100)
-			 1)
-		     (or (eq (char-after (+ (point) size -1)) ?-)
-			 (eq (char-after (+ (point) size -2)) ?-)))
-		;; This is a range so just replace the end part.
-		(delete-char size)
-	      ;; Insert a comma with the preferred number of spaces.
-	      (insert
-	       (save-excursion
-		 (if (re-search-backward "[0-9]\\( *, *\\)[0-9]"
-					 (line-beginning-position) t)
-		     (match-string 1)
-		   ", ")))
-	      ;; If people use the '91 '92 '93 scheme, do that as well.
-	      (if (eq (char-after (+ (point) size -3)) ?')
-		  (insert ?')))
-	    ;; Finally insert the new year.
-	    (insert (substring copyright-current-year size)))))))
+  (let ((copyright-end (point)))
+    (setq copyright-current-year (format-time-string "%Y"))
+    (unless (string= (buffer-substring (- (match-end 3) 2) (match-end 3))
+		     (substring copyright-current-year -2))
+      (if (or noquery
+              ;; ‘switch-to-buffer’ can pop up a new frame, or use
+              ;; another window, so preserve the current window
+              ;; config.
+              (save-window-excursion
+	        ;; Fixes some point-moving oddness (bug#2209, bug#29935).
+	        (save-excursion
+	          (switch-to-buffer (current-buffer))
+                  ;; Ensure the copyright line is displayed;
+                  ;; switch-to-buffer has moved point to where it was
+                  ;; when this buffer was last displayed in this
+                  ;; window.
+                  (goto-char copyright-end)
+		  (y-or-n-p (if replace
+			        (concat "Replace copyright year(s) by "
+				        copyright-current-year "? ")
+			      (concat "Add " copyright-current-year
+				      " to copyright? "))))))
+	  (if replace
+	      (replace-match copyright-current-year t t nil 3)
+	    (let ((size (save-excursion (skip-chars-backward "0-9"))))
+	      (if (and (eq (% (- (string-to-number copyright-current-year)
+			         (string-to-number (buffer-substring
+						    (+ (point) size)
+						    (point))))
+			      100)
+			   1)
+		       (or (eq (char-after (+ (point) size -1)) ?-)
+			   (eq (char-after (+ (point) size -2)) ?-)))
+		  ;; This is a range so just replace the end part.
+		  (delete-char size)
+	        ;; Insert a comma with the preferred number of spaces.
+	        (insert
+	         (save-excursion
+		   (if (re-search-backward "[0-9]\\( *, *\\)[0-9]"
+					   (line-beginning-position) t)
+		       (match-string 1)
+		     ", ")))
+	        ;; If people use the '91 '92 '93 scheme, do that as well.
+	        (if (eq (char-after (+ (point) size -3)) ?')
+		    (insert ?')))
+	      ;; Finally insert the new year.
+	      (insert (substring copyright-current-year size))))
+        ))))
 
 ;;;###autoload
 (defun copyright-update (&optional arg interactivep)

Ok to commit?

--
-- Stephe

Re: fix for bug#29935 copyright-update inserts year at random places

[Stefan Monnier]

>> Why does this need to use save-window-excursion?
> 'switch-to-buffer' can pop up a new frame, or use a different window,
> depending on various settings, so save-window-excursion is needed. 

Indeed, but note that if it pops up a new frame, save-window-excursion
will be of no help to "undo" this effect (which really can't be undone
in general, since to pop up that frame, the user may have had to place
the new frame).

Also switch-to-buffer is kind of ambiguous: do you really want to only
affect the currently selected window, or do you really want to display
the specified buffer (sometimes switch-to-buffer has to choose between
those two options and it doesn't know what the caller wanted).  I think
here we care more about displaying the specified buffer than about only
affecting the selected window, so we should probably use
`pop-to-buffer(-same-window)` instead.


        Stefan

Re: fix for bug#29935 copyright-update inserts year at random places

[Stephen Leake]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> Why does this need to use save-window-excursion?
>> 'switch-to-buffer' can pop up a new frame, or use a different window,
>> depending on various settings, so save-window-excursion is needed. 
>
> Indeed, but note that if it pops up a new frame, save-window-excursion
> will be of no help to "undo" this effect (which really can't be undone
> in general, since to pop up that frame, the user may have had to place
> the new frame).

Right. I changed copyright-update-year to use pop-to-buffer, and set
pop-up-frames to t; update-copyright-year popped up a new frame, which
was not minimized or closed.

Using pop-to-buffer-same-window did not pop up a new frame, which is
preferable here, I think, even for people who normally have
pop-up-frames t.

I think there are still situations where pop-to-buffer-same-window can
change the window configuration (if it is invoked from a dedicated
window, for example), so save-window-excursion is still needed.

> Also switch-to-buffer is kind of ambiguous: do you really want to only
> affect the currently selected window, or do you really want to display
> the specified buffer (sometimes switch-to-buffer has to choose between
> those two options and it doesn't know what the caller wanted).  I think
> here we care more about displaying the specified buffer than about only
> affecting the selected window, so we should probably use
> `pop-to-buffer(-same-window)` instead.

pop-to-buffer-same-window works, and is a lower-level function, so it
makes sense here.

-- 
-- Stephe

Re: fix for bug#29935 copyright-update inserts year at random places

[Stefan Monnier]

> I think there are still situations where pop-to-buffer-same-window can
> change the window configuration (if it is invoked from a dedicated
> window, for example), so save-window-excursion is still needed.

In my config, pop-to-buffer-same-window will often/usually pop up a new
frame (unless the buffer is already displayed somewhere, in which case
it deiconifies and/or raises that frame), "so save-window-excursion is
still needed^H^H^H^H^H^H^ineffective".

This said, I actually do like that the buffer is shown in its
own frame.  It would be nice to re-iconify the frame afterwards, tho.


        Stefan

Re: fix for bug#29935 copyright-update inserts year at random places

[Stephen Leake]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I think there are still situations where pop-to-buffer-same-window can
>> change the window configuration (if it is invoked from a dedicated
>> window, for example), so save-window-excursion is still needed.
>
> In my config, pop-to-buffer-same-window will often/usually pop up a new
> frame (unless the buffer is already displayed somewhere, in which case
> it deiconifies and/or raises that frame), "so save-window-excursion is
> still needed^H^H^H^H^H^H^ineffective".
>
> This said, I actually do like that the buffer is shown in its
> own frame.  

So using pop-to-buffer-same-window, rather than pop-to-buffer, is ok for you.

> It would be nice to re-iconify the frame afterwards, tho.

Apparently completion (or maybe read-from-minibuffer) does re-iconify a
popped up frame (that happened once in my experiments). I looked thru
the C code for read-from-minibuffer, and it has comments that talk about
preserving the frame state, but I did not see where it minimizes the
frame on return. So I don't know how to do that from elisp in
copyright-update-year.

-- 
-- Stephe

Re: fix for bug#29935 copyright-update inserts year at random places

[Stefan Monnier]

>> This said, I actually do like that the buffer is shown in its
>> own frame.  
> So using pop-to-buffer-same-window, rather than pop-to-buffer, is ok for you.

Yes.

>> It would be nice to re-iconify the frame afterwards, tho.
> Apparently completion (or maybe read-from-minibuffer) does re-iconify a
> popped up frame (that happened once in my experiments). I looked thru
> the C code for read-from-minibuffer, and it has comments that talk about
> preserving the frame state, but I did not see where it minimizes the
> frame on return. So I don't know how to do that from elisp in
> copyright-update-year.

I think rather than save-window-excursion, it uses a kind of
"un-pop-to-buffer".  Can't remember if it's bury-buffer or what, tho.


        Stefan

Re: fix for bug#29935 copyright-update inserts year at random places

[martin rudalics]

 > Apparently completion (or maybe read-from-minibuffer) does re-iconify a
 > popped up frame (that happened once in my experiments). I looked thru
 > the C code for read-from-minibuffer, and it has comments that talk about
 > preserving the frame state, but I did not see where it minimizes the
 > frame on return. So I don't know how to do that from elisp in
 > copyright-update-year.

There is one function `frame-auto-hide-function' and two frame
parameters `auto-hide-function' and `minibuffer-exit' which can be
useful in this context.

martin

Re: fix for bug#29935 copyright-update inserts year at random places

[Stefan Monnier]

>> Apparently completion (or maybe read-from-minibuffer) does re-iconify a
>> popped up frame (that happened once in my experiments). I looked thru
>> the C code for read-from-minibuffer, and it has comments that talk about
>> preserving the frame state, but I did not see where it minimizes the
>> frame on return. So I don't know how to do that from elisp in
>> copyright-update-year.
>
> There is one function `frame-auto-hide-function' and two frame
> parameters `auto-hide-function' and `minibuffer-exit' which can be
> useful in this context.

But this is a fairly normal/common need: display a buffer temporarily.
So we should have a "canned" answer.
I'm thinking of something like

    (let ((x (temporary-display-buffer BUF)))

    +

    (temporary-undisplay-buffer x)

where hopefully this would handle the case where
temporary-display-buffer needs to use a separate frame, as well as the
case where BUF is already displayed somewhere.


        Stefan

Re: fix for bug#29935 copyright-update inserts year at random places

[martin rudalics]

 > But this is a fairly normal/common need: display a buffer temporarily.
 > So we should have a "canned" answer.
 > I'm thinking of something like
 >
 >      (let ((x (temporary-display-buffer BUF)))

What kind of object would "x" be?

 >      +
 >
 >      (temporary-undisplay-buffer x)
 >
 > where hopefully this would handle the case where
 > temporary-display-buffer needs to use a separate frame, as well as the
 > case where BUF is already displayed somewhere.

If "x" is a window, then `quit-restore-window' should know how to deal
with it.  So please tell what's missing in `with-temp-buffer-window'.

martin

Re: fix for bug#29935 copyright-update inserts year at random places

[Stefan Monnier]

>> But this is a fairly normal/common need: display a buffer temporarily.
>> So we should have a "canned" answer.
>> I'm thinking of something like
>>
>>      (let ((x (temporary-display-buffer BUF)))
>
> What kind of object would "x" be?

The kind of object that carries the necessary information from
temporary-display-buffer to temporary-undisplay-buffer so that they
together DTRT.

>>      (temporary-undisplay-buffer x)
>> where hopefully this would handle the case where
>> temporary-display-buffer needs to use a separate frame, as well as the
>> case where BUF is already displayed somewhere.
> If "x" is a window, then `quit-restore-window' should know how to deal
> with it.

If `x` is just a window, how does quit-restore-window know that we want
to hide BUF (and not some other buffer that happens to be displayed
in the window once we get to quit-restore-window)?

If `x` is just a window, how does quit-restore-window know whether BUF was
already displayed in that window before temporary-undisplay-buffer was
called (in order to decide whether to change the window's buffer or not)?

> So please tell what's missing in `with-temp-buffer-window'.

It forces scoping.  IOW it can't be used when the
temporary-undisplay-buffer part needs to be done at some arbitrary later
time (e.g. after the user has run a bunch of commands).


        Stefan

Re: fix for bug#29935 copyright-update inserts year at random places

[Stephen Leake]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> But this is a fairly normal/common need: display a buffer temporarily.
>>> So we should have a "canned" answer.
>>> I'm thinking of something like
>>>
>>>      (let ((x (temporary-display-buffer BUF)))
>>
>> What kind of object would "x" be?
>
> The kind of object that carries the necessary information from
> temporary-display-buffer to temporary-undisplay-buffer so that they
> together DTRT.
>
>>>      (temporary-undisplay-buffer x)
>>> where hopefully this would handle the case where
>>> temporary-display-buffer needs to use a separate frame, as well as the
>>> case where BUF is already displayed somewhere.
>> If "x" is a window, then `quit-restore-window' should know how to deal
>> with it.
>
> If `x` is just a window, how does quit-restore-window know that we want
> to hide BUF (and not some other buffer that happens to be displayed
> in the window once we get to quit-restore-window)?
>
> If `x` is just a window, how does quit-restore-window know whether BUF was
> already displayed in that window before temporary-undisplay-buffer was
> called (in order to decide whether to change the window's buffer or not)?
>
>> So please tell what's missing in `with-temp-buffer-window'.
>
> It forces scoping.  IOW it can't be used when the
> temporary-undisplay-buffer part needs to be done at some arbitrary later
> time (e.g. after the user has run a bunch of commands).

In the context of `copyright-update-year', something like
`save-frame-excursion' is what we want, at least given the current
design.

`with-temp-buffer-window' erases the buffer, and binds
`standard-output' to it, which we certainly do not want.

It uses `quit-restore-window', which looks useful, but I think it would
work best in combination with a different way of displaying the buffer
than `save-excursion pop-to-buffer'. Perhaps a new macro
`with-temp-window' is what we want.

This thread is nominally about patching emacs-26 to fix a bug. The patch
I've posted is a minimal fix; adding code or redesigning to handle frames
better is feature-creep, so it should be done on master.

-- 
-- Stephe

Re: fix for bug#29935 copyright-update inserts year at random places

[martin rudalics]

 > If `x` is just a window, how does quit-restore-window know that we want
 > to hide BUF (and not some other buffer that happens to be displayed
 > in the window once we get to quit-restore-window)?
 >
 > If `x` is just a window, how does quit-restore-window know whether BUF was
 > already displayed in that window before temporary-undisplay-buffer was
 > called (in order to decide whether to change the window's buffer or not)?

`quit-restore-window' remembers what buffer the window it quits
displayed before calling `with-temp-buffer-window' and also the
respective `window-start' and `window-point' positions, sometimes also
the window's size.

 >> So please tell what's missing in `with-temp-buffer-window'.
 >
 > It forces scoping.

Where and how?  IIUC your

     (let ((x (temporary-display-buffer BUF)))

     +

     (temporary-undisplay-buffer x)

would force scoping (and is therefore IMHO not useful for our purpose)
but `quit-restore-window' doesn't rely on any scoping.

 > IOW it can't be used when the
 > temporary-undisplay-buffer part needs to be done at some arbitrary later
 > time (e.g. after the user has run a bunch of commands).

Please show me an example.  `with-temp-buffer-window' was explicitly
designed to handle what you describe here so if it doesn't for you it
clearly failed to achieve its aim.

martin

Re: fix for bug#29935 copyright-update inserts year at random places

[martin rudalics]

 > It uses `quit-restore-window', which looks useful, but I think it would
 > work best in combination with a different way of displaying the buffer
 > than `save-excursion pop-to-buffer'. Perhaps a new macro
 > `with-temp-window' is what we want.

The information required for `quit-restore-window' is set up by
`display-buffer'.  What else do you need?  Have you read the section
"Quitting Windows" in the Elisp manual?

martin

Re: fix for bug#29935 copyright-update inserts year at random places

[Eli Zaretskii]

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Date: Mon, 08 Jan 2018 10:56:59 -0600
> 
> This thread is nominally about patching emacs-26 to fix a bug. The patch
> I've posted is a minimal fix; adding code or redesigning to handle frames
> better is feature-creep, so it should be done on master.

I think the bug happens because we use save-window-excursion, and
switch buffers inside it.  The cure should be not to do what hurts.
Can this be done in this case?  If not, why not?  And why do we use
save-window-excursion in the first place? that's not the usual way to
display a buffer whose contents is referenced in an error message or a
prompt.

Thanks.

Re: fix for bug#29935 copyright-update inserts year at random places

[Stefan Monnier]

> `quit-restore-window' remembers what buffer the window it quits
> displayed before calling `with-temp-buffer-window' and also the
> respective `window-start' and `window-point' positions, sometimes also
> the window's size.

So it also remembers if the previous buffer was the same buffer?
Sounds like it would do the job, then.

Except that with-temp-buffer-window starts by erasing the buffer,
whereas here the buffer is already ready to be displayed (it's a file
buffer).

>>> So please tell what's missing in `with-temp-buffer-window'.
>> It forces scoping.
> Where and how?

I guess I was confused by the `with` in the name.

> IIUC your
>
>     (let ((x (temporary-display-buffer BUF)))
>
>     +
>
>     (temporary-undisplay-buffer x)
>
> would force scoping

No: the `x` could be stored anywhere you like.

> but `quit-restore-window' doesn't rely on any scoping.

Good.  So we just need to extract the "display-buffer" part of
with-temp-buffer-window?


        Stefan

Re: fix for bug#29935 copyright-update inserts year at random places

[martin rudalics]

 > Good.  So we just need to extract the "display-buffer" part of
 > with-temp-buffer-window?

I still don't understand why plain `display-buffer' (or
`display-buffer-same-window) shouldn't work here.

martin

Re: fix for bug#29935 copyright-update inserts year at random places

[Stefan Monnier]

>> Good.  So we just need to extract the "display-buffer" part of
>> with-temp-buffer-window?
> I still don't understand why plain `display-buffer' (or
> `display-buffer-same-window) shouldn't work here.

Maybe it does.  I don't know.  My question was simply: what are the two
functions that should be used to display and then undisplay a buffer?
And more importantly, "everyone" should know it, so this pair should be
"advertized" somehow (e.g. in the manual where we talk about
save-window-excursion and about display-buffer?).


        Stefan

Re: fix for bug#29935 copyright-update inserts year at random places

[martin rudalics]

 > Maybe it does.  I don't know.  My question was simply: what are the two
 > functions that should be used to display and then undisplay a buffer?
 > And more importantly, "everyone" should know it, so this pair should be
 > "advertized" somehow (e.g. in the manual where we talk about
 > save-window-excursion and about display-buffer?).

We have an entire section in the Elisp manual entitled

* Quitting Windows::        How to restore the state prior to displaying a
                               buffer.

Can you tell me why this is not sufficient?

martin

Re: fix for bug#29935 copyright-update inserts year at random places

[Stephen Leake]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stephen Leake <stephen_leake@stephe-leake.org>
>> Date: Mon, 08 Jan 2018 10:56:59 -0600
>> 
>> This thread is nominally about patching emacs-26 to fix a bug. The patch
>> I've posted is a minimal fix; adding code or redesigning to handle frames
>> better is feature-creep, so it should be done on master.
>
> I think the bug happens because we use save-window-excursion, and
> switch buffers inside it.  The cure should be not to do what hurts.
> Can this be done in this case?  If not, why not?  And why do we use
> save-window-excursion in the first place? that's not the usual way to
> display a buffer whose contents is referenced in an error message or a
> prompt.

I posted an update on debbugs, giving a recipe for reproducing the bug,
and giving the results of various suggested fixes.

I tried deleting save-window-excursion; that does not fix the bug.

I don't have a rationale for the current design of the function, but I
think redesigning it should be done on master, not emacs-26.

-- 
-- Stephe