bug#27639: 25.2; Fix syntax for minor mode enabling in dir locals in manual

bug#27639: 25.2; Fix syntax for minor mode enabling in dir locals in manual

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 895 bytes --]

Hello,

I recently stumbled across this Reddit post[1] where a user is trying to
enable a minor mode in .dir-locals.el. But he was doing that by setting the
mode name to `t'.

We generally(?) enable minor modes by doing (mode . MINOR) in
.dir-locals.el.

So I thought of looking up the Emacs manual to check that:

Here's an example of a .dir-locals.el file:

((nil . ((indent-tabs-mode . t)
        (fill-column . 80)))
<snip>

This sets ‘indent-tabs-mode’ and fill-column for any file in the directory
tree.

That would lead the readers to think that that's how *any* minor mode can
be enabled from .dir-locals.el.

I think that example should be fixed by using a minor mode example that can
be enabled using the (mode . MINOR) syntax.

[1]:
https://www.reddit.com/r/emacs/comments/6malb6/disable_warning_message_when_enabling_minor_modes/
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1328 bytes --]

bug#27639: 25.2; Fix syntax for minor mode enabling in dir locals in manual

[Kaushal Modi]

[-- Attachment #1.1: Type: text/plain, Size: 157 bytes --]

Hello,

I have attached this patch based off emacs-25 branch that clarifies the
documentation about minor mode enabling in .dir-locals.el.
-- 

Kaushal Modi

[-- Attachment #1.2: Type: text/html, Size: 312 bytes --]

[-- Attachment #2: 0001-Add-documentation-on-enabling-minor-modes-using-.dir.patch --]
[-- Type: application/octet-stream, Size: 2203 bytes --]

From dcae725b40c7200c0633d3594533028994593793 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Mon, 10 Jul 2017 18:31:42 -0400
Subject: [PATCH] Add documentation on enabling minor modes using
 .dir-locals.el

* doc/emacs/custom.texi (Directory Variables): Add an example that
shows how a minor mode can be enabled in a .dir-local.el (Bug#27639).
Clarify that indent-tabs-mode is not a minor mode.
---
 doc/emacs/custom.texi | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 51992ea028..a174f40f2b 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -1323,7 +1323,8 @@ Directory Variables
 
 @example
 ((nil . ((indent-tabs-mode . t)
-         (fill-column . 80)))
+         (fill-column . 80)
+         (mode . auto-fill)))
  (c-mode . ((c-file-style . "BSD")
             (subdirs . nil)))
  ("src/imported"
@@ -1332,13 +1333,16 @@ Directory Variables
 @end example
 
 @noindent
-This sets @samp{indent-tabs-mode} and @code{fill-column} for any file
-in the directory tree, and the indentation style for any C source
-file.  The special @code{subdirs} element is not a variable, but a
-special keyword which indicates that the C mode settings are only to
-be applied in the current directory, not in any subdirectories.
-Finally, it specifies a different @file{ChangeLog} file name for any
-file in the @file{src/imported} subdirectory.
+This sets the variables @samp{indent-tabs-mode} and @code{fill-column}
+for any file in the directory tree, and the indentation style for any
+C source file.  The special @code{mode} element specifies the minor
+mode to be enabled.  So @code{(mode . auto-fill)} specifies that the
+minor mode @code{auto-fill-mode} needs to be enabled.  The special
+@code{subdirs} element is not a variable, but a special keyword which
+indicates that the C mode settings are only to be applied in the
+current directory, not in any subdirectories.  Finally, it specifies a
+different @file{ChangeLog} file name for any file in the
+@file{src/imported} subdirectory.
 
 @findex add-dir-local-variable
 @findex delete-dir-local-variable
-- 
2.13.0

bug#27639: 25.2; Fix syntax for minor mode enabling in dir locals in manual

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Mon, 10 Jul 2017 11:55:58 +0000
> 
> I recently stumbled across this Reddit post[1] where a user is trying to enable a minor mode in .dir-locals.el.
> But he was doing that by setting the mode name to `t'. 

indent-tabs-mode is the mode variable, so setting it to t is not a
mistake.

> We generally(?) enable minor modes by doing (mode . MINOR) in .dir-locals.el. 

You could do it either way.  Also, some modes don't have such a
variable, so only the latter way will work.

But why do you think we should only advertise one method, but not the
other?  There's no magic here, just legitimate use of public
documented variables: 'mode' is a variable and so is 'indent-tabs-mode'.

So I think I don't get your motivation for the report and for the
patch you provided.

Thanks.

bug#27639: 25.2; Fix syntax for minor mode enabling in dir locals in manual

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 2984 bytes --]

Hi Eli,

Thanks for reviewing this.

On Tue, Jul 11, 2017 at 12:38 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Kaushal Modi <kaushal.modi@gmail.com>
> > Date: Mon, 10 Jul 2017 11:55:58 +0000
> >
> > I recently stumbled across this Reddit post[1] where a user is trying to
> enable a minor mode in .dir-locals.el.
> > But he was doing that by setting the mode name to `t'.
>
> indent-tabs-mode is the mode variable, so setting it to t is not a
> mistake.
>

The mistake (as explained in the Reddit post) was setting a 'custom-mode'
to t and expecting that that would rightaway enable that 'custom-mode'
minor mode. The OP on that Reddit thread later confirms[2] that doing (mode
. custom) worked easily, while trying to do (custom-mode . t) needed him to
mark that var as safe.


> > We generally(?) enable minor modes by doing (mode . MINOR) in
> .dir-locals.el.
>
> You could do it either way.  Also, some modes don't have such a
> variable, so only the latter way will work.
>

Almost all the user minor modes are created using define-minor-mode. At
least I have not to come across one where that's not the case in the user
land.

But why do you think we should only advertise one method, but not the
> other?


From (emacs) Minor Modes

=====
   Most minor modes also have a "mode variable", with the same name as
the mode command.  Its value is non-‘nil’ if the mode is enabled, and
‘nil’ if it is disabled.  In general, you should not try to enable or
disable the mode by changing the value of the mode variable directly in
Lisp; you should run the mode command instead.

(Reading further: Does below apply to setting minor mode variables via
.dir-locals.el?)

However, setting the
mode variable through the Customize interface (*note Easy
Customization::) will always properly enable or disable the mode, since
Customize automatically runs the mode command for you.
=====

In any case, as a user, I find it clearer to understand that you must never
enable minor modes by just setting their var; you should call that minor
mode fn instead. That realization also lead me to summarize minor mode
enabling conventions here[1] few years ago.

Also, if a user uses the (mode . minor) convention instead of (minor-mode .
t), they do not have to worry about declaring them safe.

There's no magic here, just legitimate use of public
> documented variables: 'mode' is a variable and so is 'indent-tabs-mode'.
>
> So I think I don't get your motivation for the report and for the
> patch you provided.


It just sets a convention that does not cause confusion (or create hurdle
like marking vars safe).

- If you want to set a plain var: do (var . value)
- If you want to enable a minor-mode: do (mode . minor)

[1]: https://emacs.stackexchange.com/a/10971/115
[2]:
https://www.reddit.com/r/emacs/comments/6malb6/disable_warning_message_when_enabling_minor_modes/dk0yxur/
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 4447 bytes --]

bug#27639: 25.2; Fix syntax for minor mode enabling in dir locals in manual

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Tue, 11 Jul 2017 16:57:32 +0000
> Cc: 27639@debbugs.gnu.org
> 
>  indent-tabs-mode is the mode variable, so setting it to t is not a
>  mistake.
> 
> The mistake (as explained in the Reddit post) was setting a 'custom-mode' to t and expecting that that would
> rightaway enable that 'custom-mode' minor mode.

There's no variable by that name, so it's little surprise that it
didn't work.

>  But why do you think we should only advertise one method, but not the
>  other? 
> 
> From (emacs) Minor Modes
> 
> =====
> Most minor modes also have a "mode variable", with the same name as
> the mode command. Its value is non-‘nil’ if the mode is enabled, and
> ‘nil’ if it is disabled. In general, you should not try to enable or
> disable the mode by changing the value of the mode variable directly in
> Lisp; you should run the mode command instead. 

That's unrelated to file-local variables, IMO.

> In any case, as a user, I find it clearer to understand that you must never enable minor modes by just setting
> their var; you should call that minor mode fn instead.

And I object to the "never" part.  This is Emacs: we should teach
users to know and understand what they are doing, not follow recipes
prepared by others like a kind of cookbook.  If the mode variable is a
simple variable, and setting it is all that it takes to turn on or off
the mode, why should we tell users it's wrong?

> Also, if a user uses the (mode . minor) convention instead of (minor-mode . t), they do not have to worry
> about declaring them safe.

Let them find that out by themselves, and see if they care.  Who
knows, we might even find a few bugs that way -- perhaps some mode
variable should have a safe-local-variable property that we failed to
specify.

IOW, I think it's wrong to try to save users from themselves.  We
should provide clear documentation which explains the pros and cons,
and then let the users decide based on knowledge, not on blindly
following advice that makes some of the methods "magic".

OK, that's my opinion.  What do others think?

bug#27639: 25.2; Fix syntax for minor mode enabling in dir locals in manual

[Stefan Kangas]

tags 27639 + wontfix
close 27639
thanks

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Kaushal Modi <kaushal.modi@gmail.com>
>> Date: Tue, 11 Jul 2017 16:57:32 +0000
>> Cc: 27639@debbugs.gnu.org
>> 
>>  indent-tabs-mode is the mode variable, so setting it to t is not a
>>  mistake.
>> 
>> The mistake (as explained in the Reddit post) was setting a 'custom-mode' to t and expecting that that would
>> rightaway enable that 'custom-mode' minor mode.
>
> There's no variable by that name, so it's little surprise that it
> didn't work.
>
>>  But why do you think we should only advertise one method, but not the
>>  other? 
>> 
>> From (emacs) Minor Modes
>> 
>> =====
>> Most minor modes also have a "mode variable", with the same name as
>> the mode command. Its value is non-‘nil’ if the mode is enabled, and
>> ‘nil’ if it is disabled. In general, you should not try to enable or
>> disable the mode by changing the value of the mode variable directly in
>> Lisp; you should run the mode command instead. 
>
> That's unrelated to file-local variables, IMO.
>
>> In any case, as a user, I find it clearer to understand that you must never enable minor modes by just setting
>> their var; you should call that minor mode fn instead.
>
> And I object to the "never" part.  This is Emacs: we should teach
> users to know and understand what they are doing, not follow recipes
> prepared by others like a kind of cookbook.  If the mode variable is a
> simple variable, and setting it is all that it takes to turn on or off
> the mode, why should we tell users it's wrong?
>
>> Also, if a user uses the (mode . minor) convention instead of (minor-mode . t), they do not have to worry
>> about declaring them safe.
>
> Let them find that out by themselves, and see if they care.  Who
> knows, we might even find a few bugs that way -- perhaps some mode
> variable should have a safe-local-variable property that we failed to
> specify.
>
> IOW, I think it's wrong to try to save users from themselves.  We
> should provide clear documentation which explains the pros and cons,
> and then let the users decide based on knowledge, not on blindly
> following advice that makes some of the methods "magic".
>
> OK, that's my opinion.  What do others think?

I agree with you Eli, and since no one else has commented in the last
2 years, I'm closing this bug report as wontfix.

Best regards,
Stefan Kangas

bug#27639: 25.2; Fix syntax for minor mode enabling in dir locals in manual

[npostavs]

Kaushal Modi <kaushal.modi@gmail.com> writes:

> I recently stumbled across this Reddit post[1] where a user is trying to
> enable a minor mode in .dir-locals.el. But he was doing that by setting the
> mode name to `t'.

The reddit thread talks about "custom-mode" which seems to be a major
mode.  Unless "custom-mode" was some kind of meta-variable for "a custom
mode that I created"?  It's a bit unclear.