[PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

* lisp/files.el (auto-save-visited-inhibit): New buffer-local
variable.
(auto-save-visited-mode): Use it.

* etc/NEWS: Document new variable.
---
 etc/NEWS      | 4 ++++
 lisp/files.el | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 765a923bf7..659695b40f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -93,6 +93,10 @@ line numbers that were previously jumped to.
 ** When 'suggest-key-bindings' is non-nil, the completion list of 'M-x'
 shows equivalent key bindings for all commands that have them.
 
++++
+** Autosaving via 'auto-save-visited-mode' can now be inhibited using
+the buffer-local variable 'auto-save-visited-inhibit'.
+
 \f
 * Changes in Specialized Modes and Packages in Emacs 28.1
 
diff --git a/lisp/files.el b/lisp/files.el
index 55a0958f54..f3ea1d083f 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -425,12 +425,17 @@ auto-save-visited-interval
          (when auto-save--timer
            (timer-set-idle-time auto-save--timer value :repeat))))
 
+(defvar-local auto-save-visited-inhibit nil
+  "If non-nil, ignore this buffer for `auto-save-visited-mode'.")
+
 (define-minor-mode auto-save-visited-mode
   "Toggle automatic saving to file-visiting buffers on or off.
 
 Unlike `auto-save-mode', this mode will auto-save buffer contents
 to the visited files directly and will also run all save-related
-hooks.  See Info node `Saving' for details of the save process."
+hooks.  See Info node `Saving' for details of the save process.
+Buffers in which the variable `auto-save-visted-mode' is non-nil
+are ignored."
   :group 'auto-save
   :global t
   (when auto-save--timer (cancel-timer auto-save--timer))
@@ -441,6 +446,7 @@ auto-save-visited-mode
            #'save-some-buffers :no-prompt
            (lambda ()
              (and buffer-file-name
+                  (not auto-save-visited-inhibit)
                   (not (and buffer-auto-save-file-name
                             auto-save-visited-file-name))))))))
 
-- 
2.26.0.rc2.310.g2932bb562d-goog

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Robert Pluim]

>>>>> On Wed,  1 Apr 2020 19:57:36 +0200, Philipp Stephani <p.stephani2@gmail.com> said:

    Philipp> * lisp/files.el (auto-save-visited-inhibit): New buffer-local
    Philipp> variable.
    Philipp> (auto-save-visited-mode): Use it.

Could you expand on why this is needed? What's stopping you from just
turning off auto-save-visited-mode in those buffers, either directly
or via local variables?

    Philipp>  Unlike `auto-save-mode', this mode will auto-save buffer contents
    Philipp>  to the visited files directly and will also run all save-related
    Philipp> -hooks.  See Info node `Saving' for details of the save process."
    Philipp> +hooks.  See Info node `Saving' for details of the save process.
    Philipp> +Buffers in which the variable `auto-save-visted-mode' is non-nil

auto-save-visited-inhibit, no?

    Philipp> +are ignored."
    Philipp>    :group 'auto-save
    Philipp>    :global t
    Philipp>    (when auto-save--timer (cancel-timer auto-save--timer))
    Philipp> @@ -441,6 +446,7 @@ auto-save-visited-mode
    Philipp>             #'save-some-buffers :no-prompt
    Philipp>             (lambda ()
    Philipp>               (and buffer-file-name
    Philipp> +                  (not auto-save-visited-inhibit)
    Philipp>                    (not (and buffer-auto-save-file-name
    Philipp>                              auto-save-visited-file-name))))))))
 
    Philipp> -- 

    Philipp> 2.26.0.rc2.310.g2932bb562d-goog

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

Am Do., 2. Apr. 2020 um 11:25 Uhr schrieb Robert Pluim <rpluim@gmail.com>:
>
> >>>>> On Wed,  1 Apr 2020 19:57:36 +0200, Philipp Stephani <p.stephani2@gmail.com> said:
>
>     Philipp> * lisp/files.el (auto-save-visited-inhibit): New buffer-local
>     Philipp> variable.
>     Philipp> (auto-save-visited-mode): Use it.
>
> Could you expand on why this is needed? What's stopping you from just
> turning off auto-save-visited-mode in those buffers, either directly
> or via local variables?

auto-save-visited-mode is a global mode, it can't be turned off for
individual buffers.

>
>     Philipp>  Unlike `auto-save-mode', this mode will auto-save buffer contents
>     Philipp>  to the visited files directly and will also run all save-related
>     Philipp> -hooks.  See Info node `Saving' for details of the save process."
>     Philipp> +hooks.  See Info node `Saving' for details of the save process.
>     Philipp> +Buffers in which the variable `auto-save-visted-mode' is non-nil
>
> auto-save-visited-inhibit, no?

Yep, thanks for spotting!

>
>     Philipp> +are ignored."
>     Philipp>    :group 'auto-save
>     Philipp>    :global t
>     Philipp>    (when auto-save--timer (cancel-timer auto-save--timer))
>     Philipp> @@ -441,6 +446,7 @@ auto-save-visited-mode
>     Philipp>             #'save-some-buffers :no-prompt
>     Philipp>             (lambda ()
>     Philipp>               (and buffer-file-name
>     Philipp> +                  (not auto-save-visited-inhibit)
>     Philipp>                    (not (and buffer-auto-save-file-name
>     Philipp>                              auto-save-visited-file-name))))))))
>
>     Philipp> --
>
>     Philipp> 2.26.0.rc2.310.g2932bb562d-goog
>
>
>


-- 
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise
erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes
weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich
bitte wissen, dass die E-Mail an die falsche Person gesendet wurde.

This e-mail is confidential. If you received this communication by
mistake, please don’t forward it to anyone else, please erase all
copies and attachments, and please let me know that it has gone to the
wrong person.

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Robert Pluim]

>>>>> On Thu, 2 Apr 2020 11:40:45 +0200, Philipp Stephani <phst@google.com> said:

    Philipp> Am Do., 2. Apr. 2020 um 11:25 Uhr schrieb Robert Pluim <rpluim@gmail.com>:
    >> 
    >> >>>>> On Wed,  1 Apr 2020 19:57:36 +0200, Philipp Stephani <p.stephani2@gmail.com> said:
    >> 
    Philipp> * lisp/files.el (auto-save-visited-inhibit): New buffer-local
    Philipp> variable.
    Philipp> (auto-save-visited-mode): Use it.
    >> 
    >> Could you expand on why this is needed? What's stopping you from just
    >> turning off auto-save-visited-mode in those buffers, either directly
    >> or via local variables?

    Philipp> auto-save-visited-mode is a global mode, it can't be turned off for
    Philipp> individual buffers.

Youʼre right, I saw the 'define-minor-mode' bit and missed that itʼs a
global minor mode.

Robert

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Štěpán Němec]

On Thu, 02 Apr 2020 11:47:09 +0200
Robert Pluim wrote:

>>>>>> On Thu, 2 Apr 2020 11:40:45 +0200, Philipp Stephani <phst@google.com> said:
>
>     Philipp> Am Do., 2. Apr. 2020 um 11:25 Uhr schrieb Robert Pluim <rpluim@gmail.com>:
>     >> 
>     >> >>>>> On Wed,  1 Apr 2020 19:57:36 +0200, Philipp Stephani <p.stephani2@gmail.com> said:
>     >> 
>     Philipp> * lisp/files.el (auto-save-visited-inhibit): New buffer-local
>     Philipp> variable.
>     Philipp> (auto-save-visited-mode): Use it.
>     >> 
>     >> Could you expand on why this is needed? What's stopping you from just
>     >> turning off auto-save-visited-mode in those buffers, either directly
>     >> or via local variables?
>
>     Philipp> auto-save-visited-mode is a global mode, it can't be turned off for
>     Philipp> individual buffers.
>
> Youʼre right, I saw the 'define-minor-mode' bit and missed that itʼs a
> global minor mode.

Still, wouldn't it be better to add the rationale to the commit message?
IMO there should always be some commentary or explanation, unless it's
just an obvious or trivial fix. I often find myself wondering "why was
this change made?" when browsing the commit history, and I'm sure I'm
not the only one.

-- 
Štěpán

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

Am Do., 2. Apr. 2020 um 11:59 Uhr schrieb Štěpán Němec <stepnem@gmail.com>:
>
> On Thu, 02 Apr 2020 11:47:09 +0200
> Robert Pluim wrote:
>
> >>>>>> On Thu, 2 Apr 2020 11:40:45 +0200, Philipp Stephani <phst@google.com> said:
> >
> >     Philipp> Am Do., 2. Apr. 2020 um 11:25 Uhr schrieb Robert Pluim <rpluim@gmail.com>:
> >     >>
> >     >> >>>>> On Wed,  1 Apr 2020 19:57:36 +0200, Philipp Stephani <p.stephani2@gmail.com> said:
> >     >>
> >     Philipp> * lisp/files.el (auto-save-visited-inhibit): New buffer-local
> >     Philipp> variable.
> >     Philipp> (auto-save-visited-mode): Use it.
> >     >>
> >     >> Could you expand on why this is needed? What's stopping you from just
> >     >> turning off auto-save-visited-mode in those buffers, either directly
> >     >> or via local variables?
> >
> >     Philipp> auto-save-visited-mode is a global mode, it can't be turned off for
> >     Philipp> individual buffers.
> >
> > Youʼre right, I saw the 'define-minor-mode' bit and missed that itʼs a
> > global minor mode.
>
> Still, wouldn't it be better to add the rationale to the commit message?
> IMO there should always be some commentary or explanation, unless it's
> just an obvious or trivial fix. I often find myself wondering "why was
> this change made?" when browsing the commit history, and I'm sure I'm
> not the only one.

Sure, I'll add something like the following:
At least for me, auto-save-visited-mode is very slow and blocks user
interaction for files visited over TRAMP. Therefore, I'd like a
mechanism to disable auto-save-visited-mode for some buffers (namely,
those visiting remote files).

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Štěpán Němec]

On Thu, 2 Apr 2020 13:15:37 +0200
Philipp Stephani wrote:

>> Still, wouldn't it be better to add the rationale to the commit message?
>> IMO there should always be some commentary or explanation, unless it's
>> just an obvious or trivial fix. I often find myself wondering "why was
>> this change made?" when browsing the commit history, and I'm sure I'm
>> not the only one.
>
> Sure, I'll add something like the following:
> At least for me, auto-save-visited-mode is very slow and blocks user
> interaction for files visited over TRAMP. Therefore, I'd like a
> mechanism to disable auto-save-visited-mode for some buffers (namely,
> those visiting remote files).

Thanks. :-)

-- 
Štěpán

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed,  1 Apr 2020 19:57:36 +0200
> Cc: Philipp Stephani <phst@google.com>
> 
> +** Autosaving via 'auto-save-visited-mode' can now be inhibited using
> +the buffer-local variable 'auto-save-visited-inhibit'.
> +
>  \f
>  * Changes in Specialized Modes and Packages in Emacs 28.1
>  
> diff --git a/lisp/files.el b/lisp/files.el
> index 55a0958f54..f3ea1d083f 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -425,12 +425,17 @@ auto-save-visited-interval
>           (when auto-save--timer
>             (timer-set-idle-time auto-save--timer value :repeat))))
>  
> +(defvar-local auto-save-visited-inhibit nil
> +  "If non-nil, ignore this buffer for `auto-save-visited-mode'.")
> +

Wouldn't it be better to have a local minor mode and a globalized
minor mode, like we do in other cases?

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Stefan Monnier]

> Wouldn't it be better to have a local minor mode and a globalized
> minor mode, like we do in other cases?

We could start with the patch below (which allows disabling the mode in
specific buffers, but lacks the rest of the code which would allow
*enabling* the mode in specific buffers).


        Stefan


diff --git a/lisp/files.el b/lisp/files.el
index 55a0958f54..5132aa456a 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -441,6 +441,7 @@ auto-save-visited-mode
            #'save-some-buffers :no-prompt
            (lambda ()
              (and buffer-file-name
+                  auto-save-visited-mode
                   (not (and buffer-auto-save-file-name
                             auto-save-visited-file-name))))))))
 

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

Am Do., 2. Apr. 2020 um 17:22 Uhr schrieb Stefan Monnier
<monnier@iro.umontreal.ca>:
>
> > Wouldn't it be better to have a local minor mode and a globalized
> > minor mode, like we do in other cases?
>
> We could start with the patch below (which allows disabling the mode in
> specific buffers, but lacks the rest of the code which would allow
> *enabling* the mode in specific buffers).
>
>
>         Stefan
>
>
> diff --git a/lisp/files.el b/lisp/files.el
> index 55a0958f54..5132aa456a 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -441,6 +441,7 @@ auto-save-visited-mode
>             #'save-some-buffers :no-prompt
>             (lambda ()
>               (and buffer-file-name
> +                  auto-save-visited-mode
>                    (not (and buffer-auto-save-file-name
>                              auto-save-visited-file-name))))))))
>

Is this really OK? All minor mode variables, including
auto-save-visited-mode, by default get the "Setting this variable
directly does not take effect" documentation string, because we want
to use minor mode variables only for checking a mode's state and
always want users to call the mode function.

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Stefan Monnier]

>> diff --git a/lisp/files.el b/lisp/files.el
>> index 55a0958f54..5132aa456a 100644
>> --- a/lisp/files.el
>> +++ b/lisp/files.el
>> @@ -441,6 +441,7 @@ auto-save-visited-mode
>>             #'save-some-buffers :no-prompt
>>             (lambda ()
>>               (and buffer-file-name
>> +                  auto-save-visited-mode
>>                    (not (and buffer-auto-save-file-name
>>                              auto-save-visited-file-name))))))))
>
> Is this really OK?

Depends what you mean by "this".  The above patch is definitely
OK, yes.  It's actually "standard operation procedure" for a minor mode
to double check its minor mode variable in timers and hooks ;-)

> All minor mode variables, including auto-save-visited-mode, by default
> get the "Setting this variable directly does not take effect"
> documentation string, because we want to use minor mode variables only
> for checking a mode's state and always want users to call the
> mode function.

Indeed the above implies suggestions to do:

    (add-hook 'foo-mode-hook
              (lambda () (setq-local auto-save-visited-mode nil)))

which doesn't go through such functions and is hence arguably "bad
style".  Especially it will likely encourage the use of plain `setq` on
the variable.

This part of what I meant by "...but lacks the rest of the code".
The rest would be something like

    (define-minor-mode auto-save-visited-local-mode ...)

which would do the `setq-local` internally and might take inspiration
from `electric-indent-local-mode`.


        Stefan

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

Am So., 5. Apr. 2020 um 16:19 Uhr schrieb Stefan Monnier
<monnier@iro.umontreal.ca>:
>
> >> diff --git a/lisp/files.el b/lisp/files.el
> >> index 55a0958f54..5132aa456a 100644
> >> --- a/lisp/files.el
> >> +++ b/lisp/files.el
> >> @@ -441,6 +441,7 @@ auto-save-visited-mode
> >>             #'save-some-buffers :no-prompt
> >>             (lambda ()
> >>               (and buffer-file-name
> >> +                  auto-save-visited-mode
> >>                    (not (and buffer-auto-save-file-name
> >>                              auto-save-visited-file-name))))))))
> >
> > Is this really OK?
>
> Depends what you mean by "this".  The above patch is definitely
> OK, yes.  It's actually "standard operation procedure" for a minor mode
> to double check its minor mode variable in timers and hooks ;-)
>
> > All minor mode variables, including auto-save-visited-mode, by default
> > get the "Setting this variable directly does not take effect"
> > documentation string, because we want to use minor mode variables only
> > for checking a mode's state and always want users to call the
> > mode function.
>
> Indeed the above implies suggestions to do:
>
>     (add-hook 'foo-mode-hook
>               (lambda () (setq-local auto-save-visited-mode nil)))
>
> which doesn't go through such functions and is hence arguably "bad
> style".  Especially it will likely encourage the use of plain `setq` on
> the variable.
>
> This part of what I meant by "...but lacks the rest of the code".
> The rest would be something like
>
>     (define-minor-mode auto-save-visited-local-mode ...)
>
> which would do the `setq-local` internally and might take inspiration
> from `electric-indent-local-mode`.
>


Wouldn't such a minor mode be confusing, because you'd have to enable
both the global and the local minor mode for the functionality to
work?

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Stefan Monnier]

>> which would do the `setq-local` internally and might take inspiration
>> from `electric-indent-local-mode`.
> Wouldn't such a minor mode be confusing, because you'd have to enable
> both the global and the local minor mode for the functionality to
> work?

This question makes me think you did not bother to look at
`electric-indent-local-mode`.


        Stefan

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

Am So., 5. Apr. 2020 um 20:49 Uhr schrieb Stefan Monnier
<monnier@iro.umontreal.ca>:
>
> >> which would do the `setq-local` internally and might take inspiration
> >> from `electric-indent-local-mode`.
> > Wouldn't such a minor mode be confusing, because you'd have to enable
> > both the global and the local minor mode for the functionality to
> > work?
>
> This question makes me think you did not bother to look at
> `electric-indent-local-mode`.
>
>

Guilty as charged :-) I guess the same approach could indeed work
here. Though the implementation of electric-indent-local mode does
look a bit weird.

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

Am So., 5. Apr. 2020 um 21:58 Uhr schrieb Philipp Stephani <phst@google.com>:
>
> Am So., 5. Apr. 2020 um 20:49 Uhr schrieb Stefan Monnier
> <monnier@iro.umontreal.ca>:
> >
> > >> which would do the `setq-local` internally and might take inspiration
> > >> from `electric-indent-local-mode`.
> > > Wouldn't such a minor mode be confusing, because you'd have to enable
> > > both the global and the local minor mode for the functionality to
> > > work?
> >
> > This question makes me think you did not bother to look at
> > `electric-indent-local-mode`.
> >
> >
>
> Guilty as charged :-) I guess the same approach could indeed work
> here. Though the implementation of electric-indent-local mode does
> look a bit weird.


How strongly do people feel about having a minor mode for this? If not
too strongly, I'd go ahead with my original proposal, which is simpler
implementation-wise.

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Eli Zaretskii]

> From: Philipp Stephani <phst@google.com>
> Date: Wed, 8 Apr 2020 13:43:20 +0200
> Cc: Philipp Stephani <p.stephani2@gmail.com>, Eli Zaretskii <eliz@gnu.org>, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
> How strongly do people feel about having a minor mode for this?

IMO, it depends on how popular the "disable locally" feature will be.
If we envision that many users will want to do that for some buffers,
then I think we'll need either a local minor mode or at least a
variable where you can specify which buffers are exempt from the
global mode.

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

Am Mi., 8. Apr. 2020 um 14:03 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <phst@google.com>
> > Date: Wed, 8 Apr 2020 13:43:20 +0200
> > Cc: Philipp Stephani <p.stephani2@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
> >       Emacs developers <emacs-devel@gnu.org>
> >
> > How strongly do people feel about having a minor mode for this?
>
> IMO, it depends on how popular the "disable locally" feature will be.
> If we envision that many users will want to do that for some buffers,
> then I think we'll need either a local minor mode or at least a
> variable where you can specify which buffers are exempt from the
> global mode.

Hmm, that variable is exactly what I'm proposing here?

-- 
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise
erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes
weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich
bitte wissen, dass die E-Mail an die falsche Person gesendet wurde.

This e-mail is confidential. If you received this communication by
mistake, please don’t forward it to anyone else, please erase all
copies and attachments, and please let me know that it has gone to the
wrong person.

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Eli Zaretskii]

> From: Philipp Stephani <phst@google.com>
> Date: Wed, 8 Apr 2020 14:19:44 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Philipp Stephani <p.stephani2@gmail.com>, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
> > IMO, it depends on how popular the "disable locally" feature will be.
> > If we envision that many users will want to do that for some buffers,
> > then I think we'll need either a local minor mode or at least a
> > variable where you can specify which buffers are exempt from the
> > global mode.
> 
> Hmm, that variable is exactly what I'm proposing here?

No, not IIUC.  You proposed a buffer-local variable that has to be set
for each buffer where the global mode should be effectively disabled.
I was talking about a global variable with regular expressions that
match names of buffers where the user wants the global mode to be
disabled.

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Stefan Monnier]

> How strongly do people feel about having a minor mode for this? If not
> too strongly, I'd go ahead with my original proposal, which is simpler
> implementation-wise.

I don't think you can do much simpler than the patch I sent and it lets
users inhibit the mode in those mode where they don't want it with:

    (add-hook 'foo-mode-hook
              (lambda () (setq-local auto-save-visited-mode nil)))

and it's "compatible" with a future addition of an actual minor mode.
As mentioned, having such "redundant checks" is common practice for
minor modes.


        Stefan

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

Am Mi., 8. Apr. 2020 um 16:58 Uhr schrieb Stefan Monnier
<monnier@iro.umontreal.ca>:
>
> > How strongly do people feel about having a minor mode for this? If not
> > too strongly, I'd go ahead with my original proposal, which is simpler
> > implementation-wise.
>
> I don't think you can do much simpler than the patch I sent and it lets
> users inhibit the mode in those mode where they don't want it with:
>
>     (add-hook 'foo-mode-hook
>               (lambda () (setq-local auto-save-visited-mode nil)))
>
> and it's "compatible" with a future addition of an actual minor mode.
> As mentioned, having such "redundant checks" is common practice for
> minor modes.
>


Didn't we already discuss this above? It's against our own
recommendations for minor modes, so we shouldn't put it into the Emacs
codebase itself.

Unless I hear strong objections, I'll push my original approach in a few days.

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Stefan Monnier]

> Didn't we already discuss this above?  It's against our own
> recommendations for minor modes, so we shouldn't put it into the Emacs
> codebase itself.

It's not.  This recommendation is just a general recommendation which
doesn't cover cases like this one.

We can definitely document that this variable can be set to nil via
`setq-local` to inhibit the mode in a particular buffer.


        Stefan

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

Am Sa., 11. Apr. 2020 um 19:11 Uhr schrieb Stefan Monnier
<monnier@iro.umontreal.ca>:
>
> > Didn't we already discuss this above?  It's against our own
> > recommendations for minor modes, so we shouldn't put it into the Emacs
> > codebase itself.
>
> It's not.  This recommendation is just a general recommendation which
> doesn't cover cases like this one.
>
> We can definitely document that this variable can be set to nil via
> `setq-local` to inhibit the mode in a particular buffer.
>
>

Fair enough, I've now done this in commit f8e99938ec.

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Philipp Stephani]

Am Do., 2. Apr. 2020 um 15:39 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Wed,  1 Apr 2020 19:57:36 +0200
> > Cc: Philipp Stephani <phst@google.com>
> >
> > +** Autosaving via 'auto-save-visited-mode' can now be inhibited using
> > +the buffer-local variable 'auto-save-visited-inhibit'.
> > +
> >
> >  * Changes in Specialized Modes and Packages in Emacs 28.1
> >
> > diff --git a/lisp/files.el b/lisp/files.el
> > index 55a0958f54..f3ea1d083f 100644
> > --- a/lisp/files.el
> > +++ b/lisp/files.el
> > @@ -425,12 +425,17 @@ auto-save-visited-interval
> >           (when auto-save--timer
> >             (timer-set-idle-time auto-save--timer value :repeat))))
> >
> > +(defvar-local auto-save-visited-inhibit nil
> > +  "If non-nil, ignore this buffer for `auto-save-visited-mode'.")
> > +
>
> Wouldn't it be better to have a local minor mode and a globalized
> minor mode, like we do in other cases?

I don't think that's possible here. We have one global timer, and with
a local minor mode we'd either need one timer per buffer or some
additional complexity to ensure that the timer is enabled if at least
one buffer has the minor mode on.
Alternatively we could have a global timer that's always on.

Re: [PATCH] Allow inhibiting 'auto-save-visited-mode' on a per-buffer basis.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sun, 5 Apr 2020 11:52:16 +0200
> Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> 
> > > +(defvar-local auto-save-visited-inhibit nil
> > > +  "If non-nil, ignore this buffer for `auto-save-visited-mode'.")
> > > +
> >
> > Wouldn't it be better to have a local minor mode and a globalized
> > minor mode, like we do in other cases?
> 
> I don't think that's possible here. We have one global timer, and with
> a local minor mode we'd either need one timer per buffer or some
> additional complexity to ensure that the timer is enabled if at least
> one buffer has the minor mode on.
> Alternatively we could have a global timer that's always on.

Indeed, we could either have some reference count in the timer to
disable it when there's no buffer that needs it, or keep it running at
all times.

Of course, if this is deemed too complicated, I guess we could go with
your original proposal, but in that case we should at least have some
comment there explaining why this mode is so different from others.

Thanks.