bug#20971: 24.4; occur-1 makes my buffer read-only

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#20971: 24.4; occur-1 makes my buffer read-only

Alex Schroeder-3
I'm using isearch to search for something, then I use M-s o to run an occur for my search term. This results in *Occur* being shown and in the buffer I'm searching to be read-only. This is confusing. I think what people intended was to make *Occur* read-only. But occur-mode already inherits from special-mode which already does (setq buffer-read-only t).

I therefore propose to simply remove the (setq buffer-read-only t) line from the end of occur-1 in replace.el. See attached patch.


fix_occur.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#20971: 24.4; occur-1 makes my buffer read-only

Eli Zaretskii
> From: Alex Schröder <[hidden email]>
> Date: Fri, 03 Jul 2015 09:21:55 +0000
>
> I'm using isearch to search for something, then I use M-s o to run an occur for
> my search term. This results in *Occur* being shown and in the buffer I'm
> searching to be read-only.

I cannot reproduce this.  The buffer I was searching doesn't become
read-only.  Are you sure it's not one of your customizations?  Can you
see that in "emacs -Q"?



Reply | Threaded
Open this post in threaded view
|

bug#20971: 24.4; occur-1 makes my buffer read-only

Alex Schroeder-3
Indeed, the problem goes away with emacs -Q. I investigated further and found that the culprit is display-buffer-function being set to popwin:display-buffer. Thus, I can load my current config and evaluate (setq display-buffer-function nil). The problem will disappear.

I still think that my patch is the right solution. The current code expects display-buffer to behave in a certain way and Emacs gives us a ton of ways to change that. Thus, we either need to explicitly set the current buffer, or we can do away with trying to make *Occur* read-only. And really, who needs that? Even if we don't set buffer-read-only, the buffer has no self-insert-command bindings, and if you try to C-k the header, it will still say "Text is read-only: #<buffer *Occur*>" because there is a read-only text-property. And why shouldn't you delete matches?

Thus, as far as I can tell, my change makes the code more robust (no longer depending on display-buffer to work in a particular way) and it has practically no drawbacks.
Reply | Threaded
Open this post in threaded view
|

bug#20971: 24.4; occur-1 makes my buffer read-only

Stefan Kangas
In reply to this post by Alex Schroeder-3
Alex Schröder <[hidden email]> writes:

> Indeed, the problem goes away with emacs -Q. I investigated further and found that the culprit is display-buffer-function
> being set to popwin:display-buffer. Thus, I can load my current config and evaluate (setq display-buffer-function nil). The
> problem will disappear.
>
> I still think that my patch is the right solution. The current code expects display-buffer to behave in a certain way and
> Emacs gives us a ton of ways to change that. Thus, we either need to explicitly set the current buffer, or we can do away
> with trying to make *Occur* read-only. And really, who needs that? Even if we don't set buffer-read-only, the buffer has no
> self-insert-command bindings, and if you try to C-k the header, it will still say "Text is read-only: #<buffer *Occur*>"
> because there is a read-only text-property. And why shouldn't you delete matches?
>
> Thus, as far as I can tell, my change makes the code more robust (no longer depending on display-buffer to work in a
> particular way) and it has practically no drawbacks.

I think *Occur* should definitely be read-only, so your first patch is
the wrong solution in my opinion.

On the other hand, I see no drawbacks with wrapping the relevant calls
in:

  (with-current-buffer occur-buf
       ...)

At the very least, that would make the intention of that code a bit more
clear.

But that entire part of the code is already wrapped in
(with-current-buffer occur-buf ...).  So why are we not in occur-buf
already?

Best regards,
Stefan Kangas



Reply | Threaded
Open this post in threaded view
|

bug#20971: 24.4; occur-1 makes my buffer read-only

Lars Ingebrigtsen
Stefan Kangas <[hidden email]> writes:

> I think *Occur* should definitely be read-only, so your first patch is
> the wrong solution in my opinion.
>
> On the other hand, I see no drawbacks with wrapping the relevant calls
> in:
>
>   (with-current-buffer occur-buf
>        ...)
>
> At the very least, that would make the intention of that code a bit more
> clear.
>
> But that entire part of the code is already wrapped in
> (with-current-buffer occur-buf ...).  So why are we not in occur-buf
> already?

I am guessing the reporter had some misbehaving code running off of
`display-buffer' that changed the current buffer:

            (display-buffer occur-buf)
            (when occur--final-pos
              (set-window-point
               (get-buffer-window occur-buf 'all-frames)
               occur--final-pos))
            (setq next-error-last-buffer occur-buf)
            (setq buffer-read-only t)
            (set-buffer-modified-p nil)
            (run-hooks 'occur-hook)))))))

So everything after displaying the buffer runs somewhere else.  We could
defensively re-wrap the rest in a with-current-buffer, but...  I think
the bug is in the code that changed the current buffer, really.

So I'm closing this bug report.

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no