bug#6241: Please make buffer-offer-save permanent local

bug#6241: Please make buffer-offer-save permanent local

[Lennart Borgman]

I think everyone expects that.

bug#6241: Please make buffer-offer-save permanent local

[Stefan Monnier]

> I think everyone expects that.

AFAIK it's been buffer-local for a while now.
Maybe you haven't updated your checkout in the last 19 years?


        Stefan

bug#6241: Please make buffer-offer-save permanent local

[Juanma Barranquero]

On Tue, May 25, 2010 at 18:49, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> AFAIK it's been buffer-local for a while now.
> Maybe you haven't updated your checkout in the last 19 years?

Lennart didn't write "buffer-local", but *permanent* local, i.e.,

(put 'buffer-offer-save 'permanent-local t)

    Juanma

bug#6241: Please make buffer-offer-save permanent local

[Stefan Monnier]

reopen 6241
thanks

>> AFAIK it's been buffer-local for a while now.
>> Maybe you haven't updated your checkout in the last 19 years?
> Lennart didn't write "buffer-local", but *permanent* local, i.e.,
> (put 'buffer-offer-save 'permanent-local t)

Oh, I see, sorry for being too trigger-happy.
Hmm... I'm not sure whether it would be good or not, and more
importantly, I'm not sure how it'd affect existing code.  Could someone
investigate how safe would be such a change, and could someone argue for
(and/or against) it?


        Stefan

bug#6241: Please make buffer-offer-save permanent local

[Lennart Borgman]

On Tue, May 25, 2010 at 8:09 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

>> (put 'buffer-offer-save 'permanent-local t)
>
> Oh, I see, sorry for being too trigger-happy.
> Hmm... I'm not sure whether it would be good or not, and more
> importantly, I'm not sure how it'd affect existing code.  Could someone
> investigate how safe would be such a change, and could someone argue for
> (and/or against) it?

First trigger-happy and then worried. Disappointment often leads to worries ;-)

Maybe it is my imagination, but I can't think of anything that can go
seriously wrong here. (Unless the code that queries the user is wrong,
but let us find those bugs in that case.)

The only thing that I believe can go very wrong is that
buffer-offer-save gets killed - in the situation that we have now.

bug#6241: Please make buffer-offer-save permanent local

[MON KEY]

> importantly, I'm not sure how it'd affect existing code.  Could someone
> investigate how safe would be such a change, and could someone argue for
> (and/or against) it?

I don't have a definitive use case where this is clearly NTRT but in
general I find these types of `globals' invasive and I appreciate the
luxury of being able to nuke them when and as I see fit. Let
third-parties put the variable as needed if that is what they/their code
requires.... esp. as they may have no idea whatsoever what _I_ will be
doing with _my_ buffers. From a user perspective it is often a burden
for me to undo this type overreaching.

--
/s_p\

bug#6241: Please make buffer-offer-save permanent local

[Lennart Borgman]

On Thu, May 27, 2010 at 3:28 AM, MON KEY <monkey@sandpframing.com> wrote:
>> importantly, I'm not sure how it'd affect existing code.  Could someone
>> investigate how safe would be such a change, and could someone argue for
>> (and/or against) it?
>
> I don't have a definitive use case where this is clearly NTRT but in
> general I find these types of `globals' invasive and I appreciate the
> luxury of being able to nuke them when and as I see fit.

It seems like you are misunderstanding what

  (put 'buffer-offer-save 'permanent-local t)

means. There is no problem still killing the local variable. You can
do that with

  (kill-local-variable 'buffer-offer-save)

just as before. The only difference is that it is not killed when you
change major mode in the buffer.


> Let
> third-parties put the variable as needed if that is what they/their code
> requires.... esp. as they may have no idea whatsoever what _I_ will be
> doing with _my_ buffers. From a user perspective it is often a burden
> for me to undo this type overreaching.

What burden do you see here?

bug#6241: Please make buffer-offer-save permanent local

[MON KEY]

On Wed, May 26, 2010 at 9:35 PM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> It seems like you are misunderstanding what
>
>  (put 'buffer-offer-save 'permanent-local t)
>
> means.

I well understand what it means.

It seems you are being hasty in your zeal to initiate this change.

> There is no problem still killing the local variable. You can
> do that with
>
>  (kill-local-variable 'buffer-offer-save)
>
> just as before.

Likewise, just as it can be killed. You/your code can just as easily
put the varibabe on the buffer.  What exactly is the big deal about
having to set the variable with an explicit:

 (put 'buffer-offer-save 'permanent-local t)

> The only difference is that it is not killed when you
> change major mode in the buffer.

This is what I understood your request to pertain to.

It is potentially problematcic.

,---- (documentation 'kill-all-local-variables)
| { ... }
|
| As a special exception, local variables whose names have a non-nil
| `permanent-local' property are not eliminated by this function.
|
| The first thing this function does is run the normal hook
| `change-major-mode-hook'.
`----

Many major mode hooks run `kill-all-local-variables' implicilty.

What you may have missed is that a good deal of them also invoke it
_explicitly_ when switching.

IMHO locals like this _should_ be killed when changing major mode.

Judging by the number of explicit calls to `kill-all-local-variables'
in lisp/progmodes/* it would seem that many major-mode package authors
may think similiarly (one can't know without asking each of them).

Lennart, why do you think `buffer-local-variables' such as
`buffer-offer-save' should not be killed with
`kill-all-local-variables' as they are now.

FWIW my impression is that _you_ need this variable to be
permanent-local w/re mumamo.  However, to the extent to which that
package (and corrolary/dependent features) are often extensively
kludged/advised I'm certainly not comfortable with extending this
variable just to accomodate that morass.

Understand I intend no offense here -- its impressive what you've been
able to accomplish w/re mumamo etc.  given Emacs' limitations in this
area :-)

Regardless, to the extent that this is your motivation for making the
request it is most definitely not the NTRT.

If you're unhappy with the means with which you are required to
set/unset this variable, write a macro and be done with it, but please
don't just request change of a very important (albeit generally
overlooked) routine that will affect _every_ major-mode just because
it impedes your coding of mumamo et al.

FWIW it seems that if you can just holdout awhile longer for full
incorporation of lexical-bind a good deal of your challenges w/re
mumamo could be addressed by taking advantage of the first-class
closures it will allow.

>
> What burden do you see here?
>

What burden did you fail to take into account in your haste?

Now whenever user code, older third-party packages, major-modes
etc. call `kill-all-local-variables' any non-nil settings of
buffer-offer-save won't go away.  Users will now be required to
explicitly remove it.

Following is an enumration of the 35 files and the ~50 functions which
explicitly call to `kill-all-local-variables' in Emacs 23.2's
lisp/progmodes:

cc-mode.el.gz -> `c-mode',
	         `c++-mode',
		 `objc-mode',
		 `java-mode',
		 `idl-mode',
		 `pike-mode',
		 `awk-mode',

ebrowse.el.gz    -> `ebrowse-tree-mode',
		    `ebrowse-electric-list-mode',
		    `ebrowse-member-mode'
		    `ebrowse-electric-position-mode',

etags.el.gz      -> `next-file'  This one is quite interesting.
gdb-ui.el.gz     -> `gdb-breakpoints-mode',
		     `gdb-frames-mode',
		     `gdb-threads-mode,
		     `gdb-memory-mode',
		     `gdb-registers-mode',
		     `gdb-locals-mode',
		     `gdb-assembler-mode'

idlwave.el.gz     -> `idlwave-mode',
		     `idlwave-display-user-catalog-widget',

idlw-shell.el.gz -> Note this comment in `idlwave-shell-mode' source:
,----
| We don't do `kill-all-local-variables' here, because this is
| done by comint.
`----

ada-mode.el.gz     -> `ada-mode'
antlr-mode.el.gz   -> `antlr-mode'
asm-mode.el.gz     -> `asm-mode'
autoconf.el.gz     -> `autoconf-mode'
compile.el.gz      -> `compilation-mode'
cperl-mode.el.gz   -> `cperl-mode'
cpp.el.gz          -> `cpp-edit-mode'
dcl-mode.el.gz     -> `dcl-mode'
delphi.el.gz       -> `delphi-mode'
f90.el.gz          -> `f90-mode'
fortran.el.gz      -> `fortran-mode'
icon.el.gz         -> icon-mode
idlw-help.el.gz    -> `idlwave-help-mode'
m4-mode.el.gz      -> `m4-mode'
make-mode.el.gz    -> `makefile-mode'
meta-mode.el.gz    -> `meta-common-initialization'
modula2.el.gz      -> `modula-2-mode'
octave-mod.el.gz   -> `octave-mode'
pascal.el.gz       -> `pascal-mode'
perl-mode.el.gz    -> `perl-mode'
prolog.el.gz       -> `prolog-mode'
ruby-mode.el.gz    -> `ruby-mode'
scheme.el.gz       -> `scheme-mode'
sh-script.el.gz    -> `sh-mode'
sql.el.gz          -> `sql-mode'
vera-mode.el.gz    -> `vera-mode'
verilog-mode.el.gz -> `verilog-mode'
vhdl-mode.el.gz    -> `vhdl-mode'
xscheme.el.gz      -> `scheme-interaction-mode'

I also suspect that there are (or will be) modes which take advantage
of asynchrous processing in conjunction with a major-mode which will
now have to worry about unsetting this variable regardless of whether
the results of the asynchrounous process succeed or not...

Explicit calls to kill-all-local-variables In Emacs 23.2's /lisp/net:

dig.elc
eudc.el.gz
eudc-hotlist.el.gz
mairix.el.gz
newst-plainview.elc
newst-treeview.el.gz
quickurl.el.gz
rcirc.el.gz
snmp-mode.el.gz
xesam.el.gz

Likewise, I imagine there are some immediate corner cases where
tramp'd buffers wouldn't appreciate buffer-offer-save being permanent
local.

I'm tiring of this enumeration... I'm sure there are many more usage
instances of `kill-all-local-variables' in lisp/* which _may_ expect that
buffer-offer-save is _not_ affected in the manner which you propose.

This doesn't even begin to take into account third party code...

Should you insist that this change be made please ask on emacs-devel
before asserting that:

 "I think everyone expects that."

I don't.

--
/s_P\

bug#6241: Please make buffer-offer-save permanent local

[Lennart Borgman]

On Thu, May 27, 2010 at 11:56 PM, MON KEY <monkey@sandpframing.com> wrote:
>
> Many major mode hooks run `kill-all-local-variables' implicilty.


All of them should.


> What you may have missed is that a good deal of them also invoke it
> _explicitly_ when switching.


That is because they were written before define-derived-mode was available.


> IMHO locals like this _should_ be killed when changing major mode.


Most buffer local variables should be killed normally because they
more or less belong to the major modes.

However or buffer local variables are not the same type of creatures.
Some of them do not belong to the major mode. They rather belong to
the buffer contents (or are a state in the user interaction with the
buffer, like editor emulator state).

I think that `buffer-offer-save' belongs to the content. It protect
the contents from being thrown away by the user of mistake. And that
is a very important function, far above major modes need. Don't you
think so?

There might be exceptions when `buffer-offer-save' should be killed
when the major mode is changed, but these can easily be fixed if we
find such cases.


> FWIW my impression is that _you_ need this variable to be
> permanent-local w/re mumamo.


This has nothing to do with mumamo, but I am aware of the different
forms of buffer local variables because of mumamo.


> I also suspect that there are (or will be) modes which take advantage
> of asynchrous processing in conjunction with a major-mode which will
> now have to worry about unsetting this variable regardless of whether
> the results of the asynchrounous process succeed or not...


Please explain more exactly in what situations and why you think they
want to kill `buffer-offer-save'. If you do that we can fix such
cases. I can't think of any myself at the moment.


> Likewise, I imagine there are some immediate corner cases where
> tramp'd buffers wouldn't appreciate buffer-offer-save being permanent
> local.


Same question as above. Exactly when and why?


> Should you insist that this change be made please ask on emacs-devel
> before asserting that:
>
>  "I think everyone expects that."
>
> I don't.


Of course, if you can be more specific in your concerns. I just don't
understand them yet.

bug#6241: Please make buffer-offer-save permanent local

[Stefan Monnier]

> I think that `buffer-offer-save' belongs to the content.  It protect
> the contents from being thrown away by the user of mistake.  And that
> is a very important function, far above major modes need.  Don't you
> think so?

That's the question, indeed.  Note that the question is only relevant
for non-file buffers (since the variable is ignored for file-buffers).

>> Likewise, I imagine there are some immediate corner cases where
>> tramp'd buffers wouldn't appreciate buffer-offer-save being
>> permanent local.

Not sure what you mean by "tramp'd buffers", but AFAICT such buffers
would most likely be file-buffers and are hence unaffected.

I'd tend to agree with Lennart, if for nothing else that it's easier to
circumvent an annoying permanent-local than it is to fake one.
I.e. if a var is permanent-local and you need it to be killed by
kill-all-local-variables, you can do something like:

  (add-hook 'change-major-mode-hook
            (lambda () (kill-local-variable 'foobar)))

whereas if it is not permanent-local and you need it to survive a change
of major-mode, you have to jump through a few more hoops (a
change-major-mode-hook to stash the variable's content somewhere, plus
an after-change-major-mode-hook to reset it after the fact, plus
additional code to handle the case when after-change-major-mode-hook is
not run, ...).

So, now that the question of "should it ideally be permanent-local?" is
resolved, the remaining question is: "what would be the impact of such
a change", which requires examining all uses of the variable and
assessing the effect of the change for each case.


        Stefan

bug#6241: Please make buffer-offer-save permanent local

[Lennart Borgman]

On Fri, May 28, 2010 at 2:27 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>
> So, now that the question of "should it ideally be permanent-local?" is
> resolved, the remaining question is: "what would be the impact of such
> a change", which requires examining all uses of the variable and
> assessing the effect of the change for each case.

I think the only really important cases is when buffer-offer-save is
set to t somewhere which is in

  ./ediff-util.el:589:      (setq buffer-offer-save t) ; ask before
killing buffer
  Seems OK.

  ./gnus/message.el:2900:  (setq buffer-offer-save t)
  ./mail/sendmail.el:654:  (setq buffer-offer-save t)
  ./mh-e/mh-folder.el:647:  (setq buffer-offer-save t)
  All those seems OK to me, but I am not sure. I am not using them.

  ./org/org-src.el:629:    (setq buffer-offer-save t)
  Seems OK.

bug#6241: Please make buffer-offer-save permanent local

[MON KEY]

On Thu, May 27, 2010 at 8:27 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> Not sure what you mean by "tramp'd buffers", but AFAICT such buffers
> would most likely be file-buffers and are hence unaffected.

howsabout for example comint when tramp'd?

>
> So, now that the question of "should it ideally be permanent-local?" is
> resolved, the remaining question is: "what would be the impact of such

Resolved... WTF?

> a change", which requires examining all uses of the variable and
> assessing the effect of the change for each case.

This is ridiculous, please poll whatever this proposed change is
becoming on emacs-devel before proceeding w/ this line of reasoning.
This is the wrong way to draw a conclusion esp. in lieu of a bug
report that was initially one sentence in length.

Its not even a bug!!!

>        Stefan

--
/s_P\

bug#6241: Please make buffer-offer-save permanent local

[MON KEY]

On Thu, May 27, 2010 at 6:25 PM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
>
> Please explain more exactly in what situations and why you think they
> want to kill `buffer-offer-save'. If you do that we can fix such
> cases. I can't think of any myself at the moment.
>

This is wrong. It is your request, you should be explaining on
emacs-devel the exact
reason, rationale and use case intended for why this change is needed.

> Same question as above. Exactly when and why?

Indeed.
What are the specific situations this change will remedy.
What is the current practice for dealing with them.
What is lost if the proposed change doesn't occur.

These are the things you should be asking in a more public forum.

--
/s_P\

bug#6241: Please make buffer-offer-save permanent local

[Lennart Borgman]

On Fri, May 28, 2010 at 6:48 AM, MON KEY <monkey@sandpframing.com> wrote:
> On Thu, May 27, 2010 at 6:25 PM, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>>
>> Please explain more exactly in what situations and why you think they
>> want to kill `buffer-offer-save'. If you do that we can fix such
>> cases. I can't think of any myself at the moment.
>>
>
> This is wrong. It is your request, you should be explaining on
> emacs-devel the exact
> reason, rationale and use case intended for why this change is needed.
>
>> Same question as above. Exactly when and why?
>
> Indeed.
> What are the specific situations this change will remedy.
> What is the current practice for dealing with them.
> What is lost if the proposed change doesn't occur.


Of course.

However I think I have explained this. Here it is again:

1) The reason for the proposed change is that people should not loose
their changes to non-file buffer that they have said they might want
to save.

2) Currently if a user has set buffer-offer-save to t for this it is
lost if the buffer major mode for some reason is changed.

3) User data is lost.

4) There might be cases where some "smart" code actually delete the
buffer local value buffer-offer-save by intention implicitly by
switching major mode. Doing this implicitly hides the purpose and is
therefore easily broken and misunderstood since major modes is
normally not at all connected with whether the buffer content should
be saved or not. Rather these two things are more orthogonal.


Could you please explain to me your reason for asking the above
questions after all I have already written before about this? Have I
been to unclear so that you did not understand? Or do you think
something is missing?


> These are the things you should be asking in a more public forum.
>
> --
> /s_P\
>

bug#6241: Please make buffer-offer-save permanent local

[Lars Magne Ingebrigtsen]

Lennart Borgman <lennart.borgman@gmail.com> writes:

>> So, now that the question of "should it ideally be permanent-local?" is
>> resolved, the remaining question is: "what would be the impact of such
>> a change", which requires examining all uses of the variable and
>> assessing the effect of the change for each case.
>
> I think the only really important cases is when buffer-offer-save is
> set to t somewhere which is in
>
>   ./ediff-util.el:589:      (setq buffer-offer-save t) ; ask before
> killing buffer
>   Seems OK.
>
>   ./gnus/message.el:2900:  (setq buffer-offer-save t)
>   ./mail/sendmail.el:654:  (setq buffer-offer-save t)
>   ./mh-e/mh-folder.el:647:  (setq buffer-offer-save t)
>   All those seems OK to me, but I am not sure. I am not using them.
>
>   ./org/org-src.el:629:    (setq buffer-offer-save t)
>   Seems OK.

I think it would make sense to make it 'permanent-local in these cases,
at least.  If there's only these use cases, then it seems like the right
thing to do.

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

bug#6241: Please make buffer-offer-save permanent local

[Stefan Monnier]

> I think it would make sense to make it 'permanent-local in these cases,
> at least.  If there's only these use cases, then it seems like the right
> thing to do.

Feel free to make it permanent-local, but then please check that all
uses are still correct.


        Stefan

bug#6241: Please make buffer-offer-save permanent local

[Lars Magne Ingebrigtsen]

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

>> I think it would make sense to make it 'permanent-local in these cases,
>> at least.  If there's only these use cases, then it seems like the right
>> thing to do.
>
> Feel free to make it permanent-local, but then please check that all
> uses are still correct.

I've now done so, and things still seem to work, so I'm checking it in.

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