* fix for bug#29935 copyright-update inserts year at random places @ 2018-01-01 23:29 Stephen Leake 2018-01-02 9:21 ` Lele Gaifax 2018-01-02 16:03 ` Eli Zaretskii 0 siblings, 2 replies; 22+ messages in thread From: Stephen Leake @ 2018-01-01 23:29 UTC (permalink / raw) To: emacs-devel 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-01 23:29 fix for bug#29935 copyright-update inserts year at random places Stephen Leake @ 2018-01-02 9:21 ` Lele Gaifax 2018-01-02 16:03 ` Eli Zaretskii 1 sibling, 0 replies; 22+ messages in thread From: Lele Gaifax @ 2018-01-02 9:21 UTC (permalink / raw) To: emacs-devel 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-01 23:29 fix for bug#29935 copyright-update inserts year at random places Stephen Leake 2018-01-02 9:21 ` Lele Gaifax @ 2018-01-02 16:03 ` Eli Zaretskii 2018-01-03 22:52 ` Stephen Leake 1 sibling, 1 reply; 22+ messages in thread From: Eli Zaretskii @ 2018-01-02 16:03 UTC (permalink / raw) To: Stephen Leake; +Cc: emacs-devel > 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-02 16:03 ` Eli Zaretskii @ 2018-01-03 22:52 ` Stephen Leake 2018-01-04 1:01 ` Stefan Monnier 0 siblings, 1 reply; 22+ messages in thread From: Stephen Leake @ 2018-01-03 22:52 UTC (permalink / raw) To: emacs-devel 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-03 22:52 ` Stephen Leake @ 2018-01-04 1:01 ` Stefan Monnier 2018-01-04 3:21 ` Stephen Leake 0 siblings, 1 reply; 22+ messages in thread From: Stefan Monnier @ 2018-01-04 1:01 UTC (permalink / raw) To: emacs-devel >> 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-04 1:01 ` Stefan Monnier @ 2018-01-04 3:21 ` Stephen Leake 2018-01-04 5:34 ` Stefan Monnier 0 siblings, 1 reply; 22+ messages in thread From: Stephen Leake @ 2018-01-04 3:21 UTC (permalink / raw) To: emacs-devel 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-04 3:21 ` Stephen Leake @ 2018-01-04 5:34 ` Stefan Monnier 2018-01-04 12:47 ` Stephen Leake 0 siblings, 1 reply; 22+ messages in thread From: Stefan Monnier @ 2018-01-04 5:34 UTC (permalink / raw) To: emacs-devel > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-04 5:34 ` Stefan Monnier @ 2018-01-04 12:47 ` Stephen Leake 2018-01-04 18:23 ` Stefan Monnier 2018-01-07 16:08 ` martin rudalics 0 siblings, 2 replies; 22+ messages in thread From: Stephen Leake @ 2018-01-04 12:47 UTC (permalink / raw) To: emacs-devel 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-04 12:47 ` Stephen Leake @ 2018-01-04 18:23 ` Stefan Monnier 2018-01-07 16:08 ` martin rudalics 1 sibling, 0 replies; 22+ messages in thread From: Stefan Monnier @ 2018-01-04 18:23 UTC (permalink / raw) To: emacs-devel >> 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-04 12:47 ` Stephen Leake 2018-01-04 18:23 ` Stefan Monnier @ 2018-01-07 16:08 ` martin rudalics 2018-01-07 17:34 ` Stefan Monnier 1 sibling, 1 reply; 22+ messages in thread From: martin rudalics @ 2018-01-07 16:08 UTC (permalink / raw) To: Stephen Leake, emacs-devel > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-07 16:08 ` martin rudalics @ 2018-01-07 17:34 ` Stefan Monnier 2018-01-08 9:53 ` martin rudalics 0 siblings, 1 reply; 22+ messages in thread From: Stefan Monnier @ 2018-01-07 17:34 UTC (permalink / raw) To: emacs-devel >> 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-07 17:34 ` Stefan Monnier @ 2018-01-08 9:53 ` martin rudalics 2018-01-08 13:15 ` Stefan Monnier 0 siblings, 1 reply; 22+ messages in thread From: martin rudalics @ 2018-01-08 9:53 UTC (permalink / raw) To: Stefan Monnier, emacs-devel > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-08 9:53 ` martin rudalics @ 2018-01-08 13:15 ` Stefan Monnier 2018-01-08 16:56 ` Stephen Leake 2018-01-08 18:18 ` martin rudalics 0 siblings, 2 replies; 22+ messages in thread From: Stefan Monnier @ 2018-01-08 13:15 UTC (permalink / raw) To: martin rudalics; +Cc: emacs-devel >> 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-08 13:15 ` Stefan Monnier @ 2018-01-08 16:56 ` Stephen Leake 2018-01-08 18:37 ` martin rudalics 2018-01-08 19:05 ` Eli Zaretskii 2018-01-08 18:18 ` martin rudalics 1 sibling, 2 replies; 22+ messages in thread From: Stephen Leake @ 2018-01-08 16:56 UTC (permalink / raw) To: emacs-devel 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-08 16:56 ` Stephen Leake @ 2018-01-08 18:37 ` martin rudalics 2018-01-08 19:05 ` Eli Zaretskii 1 sibling, 0 replies; 22+ messages in thread From: martin rudalics @ 2018-01-08 18:37 UTC (permalink / raw) To: Stephen Leake, emacs-devel > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-08 16:56 ` Stephen Leake 2018-01-08 18:37 ` martin rudalics @ 2018-01-08 19:05 ` Eli Zaretskii 2018-01-11 16:48 ` Stephen Leake 1 sibling, 1 reply; 22+ messages in thread From: Eli Zaretskii @ 2018-01-08 19:05 UTC (permalink / raw) To: Stephen Leake; +Cc: emacs-devel > 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-08 19:05 ` Eli Zaretskii @ 2018-01-11 16:48 ` Stephen Leake 0 siblings, 0 replies; 22+ messages in thread From: Stephen Leake @ 2018-01-11 16:48 UTC (permalink / raw) To: emacs-devel 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-08 13:15 ` Stefan Monnier 2018-01-08 16:56 ` Stephen Leake @ 2018-01-08 18:18 ` martin rudalics 2018-01-08 22:18 ` Stefan Monnier 1 sibling, 1 reply; 22+ messages in thread From: martin rudalics @ 2018-01-08 18:18 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-08 18:18 ` martin rudalics @ 2018-01-08 22:18 ` Stefan Monnier 2018-01-09 9:42 ` martin rudalics 0 siblings, 1 reply; 22+ messages in thread From: Stefan Monnier @ 2018-01-08 22:18 UTC (permalink / raw) To: martin rudalics; +Cc: emacs-devel > `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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-08 22:18 ` Stefan Monnier @ 2018-01-09 9:42 ` martin rudalics 2018-01-09 13:07 ` Stefan Monnier 0 siblings, 1 reply; 22+ messages in thread From: martin rudalics @ 2018-01-09 9:42 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-09 9:42 ` martin rudalics @ 2018-01-09 13:07 ` Stefan Monnier 2018-01-10 10:19 ` martin rudalics 0 siblings, 1 reply; 22+ messages in thread From: Stefan Monnier @ 2018-01-09 13:07 UTC (permalink / raw) To: martin rudalics; +Cc: emacs-devel >> 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix for bug#29935 copyright-update inserts year at random places 2018-01-09 13:07 ` Stefan Monnier @ 2018-01-10 10:19 ` martin rudalics 0 siblings, 0 replies; 22+ messages in thread From: martin rudalics @ 2018-01-10 10:19 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2018-01-11 16:48 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-01 23:29 fix for bug#29935 copyright-update inserts year at random places Stephen Leake 2018-01-02 9:21 ` Lele Gaifax 2018-01-02 16:03 ` Eli Zaretskii 2018-01-03 22:52 ` Stephen Leake 2018-01-04 1:01 ` Stefan Monnier 2018-01-04 3:21 ` Stephen Leake 2018-01-04 5:34 ` Stefan Monnier 2018-01-04 12:47 ` Stephen Leake 2018-01-04 18:23 ` Stefan Monnier 2018-01-07 16:08 ` martin rudalics 2018-01-07 17:34 ` Stefan Monnier 2018-01-08 9:53 ` martin rudalics 2018-01-08 13:15 ` Stefan Monnier 2018-01-08 16:56 ` Stephen Leake 2018-01-08 18:37 ` martin rudalics 2018-01-08 19:05 ` Eli Zaretskii 2018-01-11 16:48 ` Stephen Leake 2018-01-08 18:18 ` martin rudalics 2018-01-08 22:18 ` Stefan Monnier 2018-01-09 9:42 ` martin rudalics 2018-01-09 13:07 ` Stefan Monnier 2018-01-10 10:19 ` martin rudalics
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git 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 read-only IMAP folder(s) and NNTP newsgroup(s).