toggle-read-only should not give compilation warnings

toggle-read-only should not give compilation warnings

[Chong Yidong]

There are lots of toggle-read-only warnings in the Emacs Lisp source
tree.  As far as I can tell, the overwhelming majority of these are all
legitimate use cases.  The typical intention is "do whatever the
`toggle-read-only' command does, then do some other stuff on top of
that," so the caveat about toggle-read-only not being used in Lisp code
is bogus.

Instead of sprinking with-no-warnings all over, I propose removing
toggle-read-only from byte-compile-interactive-only-functions.  Any
objections?

Re: toggle-read-only should not give compilation warnings

[Samuel Bronson]

On Jul 11, 2012, at 12:11 AM, Chong Yidong wrote:

> There are lots of toggle-read-only warnings in the Emacs Lisp source
> tree.  As far as I can tell, the overwhelming majority of these are  
> all
> legitimate use cases.  The typical intention is "do whatever the
> `toggle-read-only' command does, then do some other stuff on top of
> that," so the caveat about toggle-read-only not being used in Lisp  
> code
> is bogus.
>
> Instead of sprinking with-no-warnings all over, I propose removing
> toggle-read-only from byte-compile-interactive-only-functions.  Any
> objections?

Maybe we need an "explain why the byte compiler warning for the next  
form is inapplicable" form?

Re: toggle-read-only should not give compilation warnings

[Achim Gratz]

Chong Yidong writes:
> Instead of sprinking with-no-warnings all over, I propose removing
> toggle-read-only from byte-compile-interactive-only-functions.  Any
> objections?

Wouldn't it be nicer to have a non-interactive interface to that
functionality, with the toggling of "view-mode" removed (which seems to
be what the warning admonishes against)?  Maybe something named
with-read-only-buffer and with-editable-buffer?


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Re: toggle-read-only should not give compilation warnings

[Glenn Morris]

Chong Yidong wrote:

> There are lots of toggle-read-only warnings in the Emacs Lisp source
> tree. 

I count 10 (excluding obsolete/). Some are probably already suppressed
though. Of these:

cedet/ede/util.el seems obviously wrong.
cedet/semantic/debug.el and vc/ediff.el are unclear to me.
gnus/smime.el seems bugged (fails to ensure a pre-existing buffer is
read-write).
vc/vc-cvs.el seems wrong.
vc/vc.el not immediately clear.

> As far as I can tell, the overwhelming majority of these are all
> legitimate use cases. The typical intention is "do whatever the
> `toggle-read-only' command does, then do some other stuff on top of
> that," so the caveat about toggle-read-only not being used in Lisp
> code is bogus.

The obvious intention is to make the buffer read-only. Often it's
unclear to me whether the author also wanted the "extra stuff" that
toggle-read-only does (which is only view-mode, isn't it?), or just
didn't think about it.

IMO, it would be better to factor out the view mode enabling piece to a
separate function to be explicitly called when necessary after setting
buffer-read-only. This would then remove the ambiguity.

Some issues caused by inappropriate use of toggle-read-only are: 7292,
10006.

Maybe on average it's right to use it as often as it is wrong. A warning
at least might make people thunk about whether they do want to use it.

Re: toggle-read-only should not give compilation warnings

[Richard Stallman]

The reason for putting toggle-read-only into that list is that it
displays a message, and programmers who don't know better would often
use it for code that wants to modify some read-only text, code that
really ought to bind inhibit-read-only.

    There are lots of toggle-read-only warnings in the Emacs Lisp source
    tree.

How many is "lots"?

      As far as I can tell, the overwhelming majority of these are all
    legitimate use cases.

That may be true -- because most of the erroneous uses were already
fixed thanks to this warning.

--
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call

Re: toggle-read-only should not give compilation warnings

[Chong Yidong]

Glenn Morris <rgm@gnu.org> writes:

> IMO, it would be better to factor out the view mode enabling piece to
> a separate function to be explicitly called when necessary after
> setting buffer-read-only.  This would then remove the ambiguity.
>
> Some issues caused by inappropriate use of toggle-read-only are: 7292,
> 10006.

toggle-read-only does two View mode related things: (i) if the buffer is
in View mode, making it writable disables View mode; and (ii) if
`view-read-only' is non-nil, making the buffer read-only enables View
mode.

As far as I can tell, (i) is generally desirable, even in Lisp calls;
otherwise the buffer ends up in an unsatisfactory state where most
self-inserting keys do the usual thing but SPC scrolls.  Bug#7292 is a
case where the Lisp caller is not set up to handle (ii) properly, and in
that case setting `buffer-read-only' is the right thing to do.  But
there are many other cases where Lisp callers really want to do the same
as C-x C-q, including obeying `view-read-only'.

As for Bug#10006, that is a plain misunderstanding about the
`buffer-read-only' variable; the reporter could have filed the same bug
with respect to an interactive invocation of `C-x C-q'.  So it doesn't
seem relevant.

> Maybe on average it's right to use it as often as it is wrong. A
> warning at least might make people thunk about whether they do want to
> use it.

A prevalance of spurious warnings tend to mask real ones, by teaching
people to ignore warnings.  So I'd like to get rid of these one way or
another---but preferably without sprinking with-no-warnings all over the
place.

RE: toggle-read-only should not give compilation warnings

[Drew Adams]

> A prevalance of spurious warnings tend to mask real ones, by teaching
> people to ignore warnings.  So I'd like to get rid of these one way or
> another---but preferably without sprinking with-no-warnings 
> all over the place.

+1 for that paragraph and everything else CY said.

Re: toggle-read-only should not give compilation warnings

[Stefan Monnier]

> A prevalance of spurious warnings tend to mask real ones, by teaching
> people to ignore warnings.  So I'd like to get rid of these one way or
> another---but preferably without sprinking with-no-warnings all over the
> place.

Once we're sure that the remaining calls to toggle-read-only are all
"correct", we should try to understand why that is and provide some
alternative function for that (and make toggle-read-only use that
function, of course).


        Stefan

Re: toggle-read-only should not give compilation warnings

[Chong Yidong]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Once we're sure that the remaining calls to toggle-read-only are all
> "correct", we should try to understand why that is and provide some
> alternative function for that (and make toggle-read-only use that
> function, of course).

It's not difficult to see why the remaining calls are correct: they want
to do the same thing as C-x C-q, either acting on a different buffer or
with a different interactive invocation.  Examples include
mode-line-toggle-read-only and Buffer-menu-toggle-read-only.

Adding another function, rather than simply using toggle-read-only, is
an unnecessary level of indirection.

Re: toggle-read-only should not give compilation warnings

[Stefan Monnier]

>> Once we're sure that the remaining calls to toggle-read-only are all
>> "correct", we should try to understand why that is and provide some
>> alternative function for that (and make toggle-read-only use that
>> function, of course).
> It's not difficult to see why the remaining calls are correct: they want
> to do the same thing as C-x C-q, either acting on a different buffer or
> with a different interactive invocation.  Examples include
> mode-line-toggle-read-only and Buffer-menu-toggle-read-only.
> Adding another function, rather than simply using toggle-read-only, is
> an unnecessary level of indirection.

Even better.


        Stefan