GNU bug report logs - #46268
27.1.91; Error in occur-rename-buffer

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46268 in the body.
You can then email your comments to 46268 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.1.91; Error in occur-rename-buffer
Date: Wed, 03 Feb 2021 10:54:24 +0200

[Message part 1 (text/plain, inline)]

Tags: patch
X-Debbugs-Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>

After evaluating (add-hook 'occur-hook 'occur-rename-buffer)
then selecting a region, and running 'M-x occur' with any string
in the active region, signals the error:

  (wrong-type-argument bufferp #<overlay from 1 to 72 in *scratch*>)

It seems the attached patch is the right way to fix this.

Also Cc:ing Stefan with a question about occur--garbage-collect-revert-args:
trying to revert the Occur buffer created from an active region
removes the source buffer name from the Occur buffer name.
For example, (after applying the attached patch) 'M-x occur'
on the active region in *scratch* creates the buffer *Occur: *scratch**.

But reverting the Occur buffer with 'g' removes the *scratch* buffer name,
and renames the Occur buffer to *Occur: *.

This is because of two lines in occur-1:

          (occur--garbage-collect-revert-args)
	  (setq occur-revert-arguments (list regexp nlines bufs))

where occur--garbage-collect-revert-args deletes the overlay
with the buffer name *scratch*, and later occur-rename-buffer
can't get the original buffer name from occur-revert-arguments.
And I don't know how to fix this.

[occur-rename-buffer-overlay.patch (text/x-diff, inline)]

diff --git a/lisp/replace.el b/lisp/replace.el
index f13d27aff8..c4c80118b4 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1544,11 +1544,17 @@ occur-rename-buffer
   (interactive "P\np")
   (with-current-buffer
       (if (eq major-mode 'occur-mode) (current-buffer) (get-buffer "*Occur*"))
-    (rename-buffer (concat "*Occur: "
-                           (mapconcat #'buffer-name
-                                      (car (cddr occur-revert-arguments)) "/")
-                           "*")
-                   (or unique-p (not interactive-p)))))
+    (rename-buffer
+     (concat "*Occur: "
+             (mapconcat (lambda (boo)
+                          (or (and (buffer-live-p boo)
+                                   (buffer-name boo))
+                              (and (overlayp boo)
+                                   (buffer-live-p (overlay-buffer boo))
+                                   (buffer-name (overlay-buffer boo)))))
+                        (car (cddr occur-revert-arguments)) "/")
+             "*")
+     (or unique-p (not interactive-p)))))
 
 ;; Region limits when `occur' applies on a region.
 (defvar occur--final-pos nil)

Message #8 received at 46268 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46268 <at> debbugs.gnu.org
Subject: Re: bug#46268: 27.1.91; Error in occur-rename-buffer
Date: Wed, 03 Feb 2021 10:05:43 -0500

> Also Cc:ing Stefan with a question about occur--garbage-collect-revert-args:

[ Side note: Not sure why, I'm not familiar with this code.  ]

> trying to revert the Occur buffer created from an active region
> removes the source buffer name from the Occur buffer name.
> For example, (after applying the attached patch) 'M-x occur'
> on the active region in *scratch* creates the buffer *Occur: *scratch**.
>
> But reverting the Occur buffer with 'g' removes the *scratch* buffer name,
> and renames the Occur buffer to *Occur: *.
>
> This is because of two lines in occur-1:
>
>           (occur--garbage-collect-revert-args)
> 	  (setq occur-revert-arguments (list regexp nlines bufs))
>
> where occur--garbage-collect-revert-args deletes the overlay
> with the buffer name *scratch*, and later occur-rename-buffer
> can't get the original buffer name from occur-revert-arguments.
> And I don't know how to fix this.

AFAICT, `occur--garbage-collect-revert-args` aims to throw away the
overlays when they're not needed, but we will need them again if we want
to `revert-buffer`, so we shouldn't call this function until we're sure
`revert-buffer` won't be called.  IOW, I think we should just remove
the above call.

[ I just took a look at the history of this code, and I see that it
  claims I'm the one who added that function and that call 2 years
  ago.  I say this is fake news  ;-)  ]


        Stefan "not sure it's a good idea to joke about the
                re-birth of nazism"

Message #11 received at 46268 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46268 <at> debbugs.gnu.org
Subject: Re: bug#46268: 27.1.91; Error in occur-rename-buffer
Date: Wed, 03 Feb 2021 10:22:50 -0500

> +             (mapconcat (lambda (boo)
> +                          (or (and (buffer-live-p boo)
> +                                   (buffer-name boo))

`buffer-name` used to be the function that provided the functionality of
`buffer-live-p`, so you can replace

    (and (buffer-live-p boo)
         (buffer-name boo))

with just

    (buffer-live-p boo)

By definition of a buffer is live iff it has a name.

> +                              (and (overlayp boo)
> +                                   (buffer-live-p (overlay-buffer boo))
> +                                   (buffer-name (overlay-buffer boo)))))

And here you can further simplify because `overlay-buffer` only returns
non-nil if the buffer is live.


        Stefan

Message #14 received at 46268 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46268 <at> debbugs.gnu.org
Subject: Re: bug#46268: 27.1.91; Error in occur-rename-buffer
Date: Wed, 03 Feb 2021 10:38:45 -0500

> AFAICT, `occur--garbage-collect-revert-args` aims to throw away the
> overlays when they're not needed, but we will need them again if we want
> to `revert-buffer`, so we shouldn't call this function until we're sure
> `revert-buffer` won't be called.  IOW, I think we should just remove
> the above call.

I now see what's going on: this call is supposed to throw away the
overlay used by the previous invocation of `occur`.  So when we revert,
we don't want to throw them away but when it's a fresh new invocation,
we do.

Maybe we should add a (unless (eq bufs (nth 2 occur-revert-arguments)))?


        Stefan

Message #17 received at 46268 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 46268 <at> debbugs.gnu.org
Subject: Re: bug#46268: 27.1.91; Error in occur-rename-buffer
Date: Wed, 03 Feb 2021 19:34:34 +0200

>> AFAICT, `occur--garbage-collect-revert-args` aims to throw away the
>> overlays when they're not needed, but we will need them again if we want
>> to `revert-buffer`, so we shouldn't call this function until we're sure
>> `revert-buffer` won't be called.  IOW, I think we should just remove
>> the above call.
>
> I now see what's going on: this call is supposed to throw away the
> overlay used by the previous invocation of `occur`.  So when we revert,
> we don't want to throw them away but when it's a fresh new invocation,
> we do.
>
> Maybe we should add a (unless (eq bufs (nth 2 occur-revert-arguments)))?

Yep, this fixed the problem:

diff --git a/lisp/replace.el b/lisp/replace.el
index f13d27aff8..babae7fed9 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1779,7 +1783,8 @@ occur-1
 			       42)
 			    (window-width))
 			 "" (occur-regexp-descr regexp))))
-          (occur--garbage-collect-revert-args)
+          (unless (eq bufs (nth 2 occur-revert-arguments))
+            (occur--garbage-collect-revert-args))
 	  (setq occur-revert-arguments (list regexp nlines bufs))
           (if (= count 0)
               (kill-buffer occur-buf)

Message #20 received at 46268 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 46268 <at> debbugs.gnu.org
Subject: Re: bug#46268: 27.1.91; Error in occur-rename-buffer
Date: Wed, 03 Feb 2021 19:36:32 +0200

>> +             (mapconcat (lambda (boo)
>> +                          (or (and (buffer-live-p boo)
>> +                                   (buffer-name boo))
>
> `buffer-name` used to be the function that provided the functionality of
> `buffer-live-p`, so you can replace
>
>     (and (buffer-live-p boo)
>          (buffer-name boo))
>
> with just
>
>     (buffer-live-p boo)
>
> By definition of a buffer is live iff it has a name.
>
>> +                              (and (overlayp boo)
>> +                                   (buffer-live-p (overlay-buffer boo))
>> +                                   (buffer-name (overlay-buffer boo)))))
>
> And here you can further simplify because `overlay-buffer` only returns
> non-nil if the buffer is live.

Maybe then this should be enough to handle all cases:

diff --git a/lisp/replace.el b/lisp/replace.el
index f13d27aff8..d320542d62 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1545,7 +1545,10 @@ occur-rename-buffer
   (with-current-buffer
       (if (eq major-mode 'occur-mode) (current-buffer) (get-buffer "*Occur*"))
     (rename-buffer (concat "*Occur: "
-                           (mapconcat #'buffer-name
+                           (mapconcat (lambda (boo)
+                                        (buffer-name (if (overlayp boo)
+                                                         (overlay-buffer boo)
+                                                       boo)))
                                       (car (cddr occur-revert-arguments)) "/")
                            "*")
                    (or unique-p (not interactive-p)))))

Message #23 received at 46268 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46268 <at> debbugs.gnu.org
Subject: Re: bug#46268: 27.1.91; Error in occur-rename-buffer
Date: Wed, 03 Feb 2021 14:41:02 -0500

> -                           (mapconcat #'buffer-name
> +                           (mapconcat (lambda (boo)
> +                                        (buffer-name (if (overlayp boo)
> +                                                         (overlay-buffer boo)
> +                                                       boo)))

LGTM,


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.