Relics of removed dir-locals-file-2 feature in pretest

Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

Hello,

I was excited to see mention of dir-locals-file-2 in NEWS, but then
recalled that that feature was added and then removed because of
performance issues.

http://git.savannah.gnu.org/cgit/emacs.git/log/?h=emacs-26&qt=grep&q=dir-locals-file-2

References to dir-locals-file-2 still need to be removed from:

km²~/downloads/:git/emacs> rg 'dir-locals-file-2'

ChangeLog.2
9419:   (dir-locals-file-2, dir-locals--all-files): Remove.
9935:   (dir-locals-file-2): New const.

etc/NEWS
360:See the variable 'dir-locals-file-2' for more information.

etc/DOC
46108:See also `dir-locals-file-2', whose values override this one's.
46109:See Info node `(elisp)Directory Local Variables' for
details.Vdir-locals-file-2

lisp/files.el
3868:See also `dir-locals-file-2', whose values override this one's.
3871:(defconst dir-locals-file-2 ".dir-locals-2.el"
-- 

Kaushal Modi

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

Re: Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

Bump. Looks like this email got missed.

On Thu, Oct 12, 2017 at 9:58 AM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> Hello,
>
> I was excited to see mention of dir-locals-file-2 in NEWS, but then
> recalled that that feature was added and then removed because of
> performance issues.
>
>
> http://git.savannah.gnu.org/cgit/emacs.git/log/?h=emacs-26&qt=grep&q=dir-locals-file-2
>
> References to dir-locals-file-2 still need to be removed from:
>
> km²~/downloads/:git/emacs> rg 'dir-locals-file-2'
>
> ChangeLog.2
> 9419:   (dir-locals-file-2, dir-locals--all-files): Remove.
> 9935:   (dir-locals-file-2): New const.
>
> etc/NEWS
> 360:See the variable 'dir-locals-file-2' for more information.
>
> etc/DOC
> 46108:See also `dir-locals-file-2', whose values override this one's.
> 46109:See Info node `(elisp)Directory Local Variables' for
> details.Vdir-locals-file-2
>
> lisp/files.el
> 3868:See also `dir-locals-file-2', whose values override this one's.
> 3871:(defconst dir-locals-file-2 ".dir-locals-2.el"
> --
>
> Kaushal Modi
>
-- 

Kaushal Modi

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

Re: Relics of removed dir-locals-file-2 feature in pretest

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Tue, 21 Nov 2017 19:56:51 +0000
> 
> Bump. Looks like this email got missed.
> 
> On Thu, Oct 12, 2017 at 9:58 AM Kaushal Modi <kaushal.modi@gmail.com> wrote:
> 
>  Hello,
> 
>  I was excited to see mention of dir-locals-file-2 in NEWS, but then recalled that that feature was added
>  and then removed because of performance issues.

It was only removed from the emacs-25 branch, not from the other
branches.

Re: Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

On Tue, Nov 21, 2017 at 3:33 PM Eli Zaretskii <eliz@gnu.org> wrote:

>
> It was only removed from the emacs-25 branch, not from the other
> branches.
>

There's a bit of confusion with that "It" in "It was removed".

I see that that dir-locals-2 feature is absent on emacs-26 branch but the
mentions are still there in NEWS, DOC and files.el (dir-locals-file
docstring and unused defconst dir-locals-file-2).
-- 

Kaushal Modi

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

Re: Relics of removed dir-locals-file-2 feature in pretest

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Tue, 21 Nov 2017 21:11:30 +0000
> Cc: emacs-devel@gnu.org
> 
> On Tue, Nov 21, 2017 at 3:33 PM Eli Zaretskii <eliz@gnu.org> wrote:
> 
>  It was only removed from the emacs-25 branch, not from the other
>  branches.
> 
> There's a bit of confusion with that "It" in "It was removed".
> 
> I see that that dir-locals-2 feature is absent on emacs-26 branch but the mentions are still there in NEWS,
> DOC and files.el (dir-locals-file docstring and unused defconst dir-locals-file-2).

You keep saying that, but I don't understand why: I still see on the
latest emacs-26 the code which you say was removed.  What am I
missing?

Re: Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

On Wed, Nov 22, 2017 at 10:19 AM Eli Zaretskii <eliz@gnu.org> wrote:

> You keep saying that, but I don't understand why: I still see on the
> latest emacs-26 the code which you say was removed.  What am I
> missing?
>

OK, I grepped for dir-locals-file-2 in the whole emacs repo, and all I get
is:

etc/NEWS:369:19:See the variable 'dir-locals-file-2' for more information.
etc/DOC:46209:11:See also `dir-locals-file-2', whose values override this
one's.
etc/DOC:46210:64:See Info node `(elisp)Directory Local Variables' for
details. Vdir-locals-file-2
lisp/files.el:3908:11:See also `dir-locals-file-2', whose values override
this one's.
lisp/files.el:3911:11:(defconst dir-locals-file-2 ".dir-locals-2.el"
ChangeLog.2:9419:3:    (dir-locals-file-2, dir-locals--all-files): Remove.
ChangeLog.2:9935:3:    (dir-locals-file-2): New const.

I see defconst dir-locals-file-2, but I don't see it being used anywhere.
So I thought that the feature was removed.

But when you said that emacs-26 still has the feature (yay!), I looked at
files.el again.. and of course the feature is there:

I see this in dir-locals--all-files:

           (file-2 (when (string-match "\\.el\\'" file-1)
                     (replace-match "-2.el" t nil file-1)))

So.. there's an issue, though of a different kind.. may be
dir-locals-file-2 mention should be removed? Or instead be used in the
code?
-- 

Kaushal Modi

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

Re: Relics of removed dir-locals-file-2 feature in pretest

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Wed, 22 Nov 2017 15:58:36 +0000
> Cc: emacs-devel@gnu.org
> 
> I see defconst dir-locals-file-2, but I don't see it being used anywhere. So I thought that the feature was
> removed.
> 
> But when you said that emacs-26 still has the feature (yay!), I looked at files.el again.. and of course the
> feature is there:
> 
> I see this in dir-locals--all-files:
> 
>            (file-2 (when (string-match "\\.el\\'" file-1)
>                      (replace-match "-2.el" t nil file-1)))
> 
> So.. there's an issue, though of a different kind.. may be dir-locals-file-2 mention should be removed? Or
> instead be used in the code? 

But it _is_ used, you just show above the code which uses it.  And
that's not the only place; see the commit to which you pointed
originally for the code which is affected by this second dir-locals
file.

So I really don't understand what is the issue that needs to be fixed.

Re: Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

On Wed, Nov 22, 2017 at 11:20 AM Eli Zaretskii <eliz@gnu.org> wrote:

> So I really don't understand what is the issue that needs to be fixed.
>

OK, at the risk of sounding very stupid.. this feature was removed and then
added again in this commit:
http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-26&id=da976cff352bdea6adc2667582a56eb4061bb5f6

Right?

The defconst dir-locals-file-2 is added in that commit, but is not used
anywhere.

Looking at the dir-locals--all-files code in that commit, from what I
understand, the let-bound file-2 derivation is hard-coded, and has nothing
to do with dir-locals-file-2:

           (file-2 (when (string-match "\\.el\\'" file-1)
                     (replace-match "-2.el" t nil file-1)))

Is that correct? So should the dir-locals-file-2 defconst and all its
mentions be removed from doc? Or should that var instead be used in
dir-locals--all-files and wherever else applicable?
-- 

Kaushal Modi

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

Re: Relics of removed dir-locals-file-2 feature in pretest

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Wed, 22 Nov 2017 16:28:18 +0000
> Cc: emacs-devel@gnu.org
> 
> OK, at the risk of sounding very stupid.. this feature was removed and then added again in this commit:
> http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-26&id=da976cff352bdea6adc2667582a56eb4061bb5f6
> 
> 
> Right?

No, it was added first, and then removed _only_ from the emacs-25
branch.  It was left on master (where it was merged from emacs-25),
and therefore it stays on today's emacs-26 and on master.

> The defconst dir-locals-file-2 is added in that commit, but is not used anywhere.
> 
> Looking at the dir-locals--all-files code in that commit, from what I understand, the let-bound file-2 derivation is
> hard-coded, and has nothing to do with dir-locals-file-2:
> 
>            (file-2 (when (string-match "\\.el\\'" file-1)
>                      (replace-match "-2.el" t nil file-1)))
> 
> Is that correct?

The facts are correct, yes.

> So should the dir-locals-file-2 defconst and all its mentions be removed from doc? Or should
> that var instead be used in dir-locals--all-files and wherever else applicable?

We could remove the defconst, but just removing it is not enough,
because that would also remove its doc string.  So we will have to do
something else in order to keep that special file name documented
(e.g., so that "M-x apropos-documentation" would find it).  I'm not
sure we should invest such an effort: after all, what's there does
work, so why fix that which ain't broken?

Re: Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

On Thu, Nov 23, 2017, 10:54 AM Eli Zaretskii <eliz@gnu.org> wrote:

> We could remove the defconst, but just removing it is not enough,
> because that would also remove its doc string.  So we will have to do
> something else in order to keep that special file name documented
> (e.g., so that "M-x apropos-documentation" would find it).  I'm not
> sure we should invest such an effort: after all, what's there does
> work, so why fix that which ain't broken?
>

.. because the documentation is incorrect. The dir-locals-file-2 variable
is useless at this point.

I am volunteering to remove references to dir-locals-file-2 everywhere and
mention the "-2.el" file instead in dir-locals-file doc-string and other
places where dir-locals-file-2 is currently mentioned.

> --

Kaushal Modi

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

Re: Relics of removed dir-locals-file-2 feature in pretest

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Thu, 23 Nov 2017 17:34:07 +0000
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
>  We could remove the defconst, but just removing it is not enough,
>  because that would also remove its doc string.  So we will have to do
>  something else in order to keep that special file name documented
>  (e.g., so that "M-x apropos-documentation" would find it).  I'm not
>  sure we should invest such an effort: after all, what's there does
>  work, so why fix that which ain't broken?
> 
> .. because the documentation is incorrect. The dir-locals-file-2 variable is useless at this point. 

Which part of it is incorrect?

> I am volunteering to remove references to dir-locals-file-2 everywhere and mention the "-2.el" file instead in
> dir-locals-file doc-string and other places where dir-locals-file-2 is currently mentioned. 

Thank you for volunteering, but I'm not yet sure we should do that.

Re: Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

On Thu, Nov 23, 2017 at 3:09 PM Eli Zaretskii <eliz@gnu.org> wrote:

>
> Which part of it is incorrect?
>

Let's say a user reads the NEWS and finds:

> * A second dir-local file (.dir-locals-2.el) is now accepted.
> See the variable 'dir-locals-file-2' for more information.

They do C-h v dir-locals-file-2 and read:

> This essentially a second file that can be used like `dir-locals-file', ..

That is incorrect! dir-locals-file-2 *cannot* be used like
`dir-locals-file'! That variable is just declared but not used anywhere
(why is it that only I think that that's a problem). A user can

Reading further in that docstring:

> See Info node `(elisp)Directory Local Variables' for details.

That node has no reference to dir-locals-file-2 variable or a mention of
the feature that .dir-locals-2.el can be used to override .dir-local.el.

Also in the doc-string of dir-locals-file:

> See also `dir-locals-file-2', whose values override this one's.

No, it *does not*! dir-locals-file-2 is not used in any code.

Thank you for volunteering, but I'm not yet sure we should do that.
>

Please consider making this change.

From your earlier email (I missed replying to this part):

> We could remove the defconst, but just removing it is not enough,
> because that would also remove its doc string.

Why is that bad? That variable is anyways not used.. so what's the point of
keeping docstring of an unused variable.

> So we will have to do
> something else in order to keep that special file name documented
> (e.g., so that "M-x apropos-documentation" would find it).  I'm not
> sure we should invest such an effort: after all, what's there does
> work,

Why not just mention that a user can just add "-2" to
(file-name-sans-extension dir-locals-file), and that file can be used to
override dir-locals-file. Mention that in the dir-locals-file docstring and
the (elisp)Directory Local Variables node.

> so why fix that which ain't broken?

The documentation is broken. It is incorrect, misleading.

And I am not just complaining, I am willing to provide a patch that removes
that defconst and updates the dir-local-file doc-string and the manual to
make this feature better discoverable. So why is that is problem? Why would
you want the 26.1 release to go out with this misdocumented feature? What
am I missing?
-- 

Kaushal Modi

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

Re: Relics of removed dir-locals-file-2 feature in pretest

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Mon, 27 Nov 2017 14:57:26 +0000
> Cc: emacs-devel@gnu.org
> 
> Let's say a user reads the NEWS and finds:
> 
> > * A second dir-local file (.dir-locals-2.el) is now accepted.
> > See the variable 'dir-locals-file-2' for more information.
> 
> They do C-h v dir-locals-file-2 and read:
> 
> > This essentially a second file that can be used like `dir-locals-file', ..
> 
> That is incorrect! dir-locals-file-2 *cannot* be used like `dir-locals-file'! That variable is just declared but not
> used anywhere (why is it that only I think that that's a problem). A user can 

The "second file" is .dir-locals-2.el, i.e. the value.

> Reading further in that docstring:
> 
> > See Info node `(elisp)Directory Local Variables' for details.
> 
> That node has no reference to dir-locals-file-2 variable or a mention of the feature that .dir-locals-2.el can be
> used to override .dir-local.el.

So maybe we need to add that to the manual.

> Also in the doc-string of dir-locals-file:
> 
> > See also `dir-locals-file-2', whose values override this one's.
> 
> No, it *does not*! dir-locals-file-2 is not used in any code.

But its (constant) value does override, doesn't it?

>  Thank you for volunteering, but I'm not yet sure we should do that.
> 
> Please consider making this change. 

Which change?  You are arguing about multiple issues, and I'm confused
about what change do you have in mind, exactly.

> From your earlier email (I missed replying to this part):
> 
> > We could remove the defconst, but just removing it is not enough,
> > because that would also remove its doc string. 
> 
> Why is that bad?

Because iut's the only place where .dir-locals-2.el is documented.

> That variable is anyways not used.. so what's the point of keeping docstring of an unused
> variable.

The doc string documents a feature.  If you remove the doc string and
do nothing else, where will that feature be documented?

> Why not just mention that a user can just add "-2" to (file-name-sans-extension dir-locals-file), and that file can
> be used to override dir-locals-file. Mention that in the dir-locals-file docstring and the (elisp)Directory Local
> Variables node.

That might work.  As I said, just removing the defconst is not enough.

> The documentation is broken. It is incorrect, misleading.

Then let's fix the documentation.

> And I am not just complaining, I am willing to provide a patch that removes that defconst and updates the
> dir-local-file doc-string and the manual to make this feature better discoverable. So why is that is problem?

I didn't say it was a problem, you misunderstood what I said.  Feel
free to submit such a patch, and thanks in advance.

Re: Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

On Mon, Nov 27, 2017 at 11:15 AM Eli Zaretskii <eliz@gnu.org> wrote:

> The "second file" is .dir-locals-2.el, i.e. the value.
>

Correct. But the current state of documentation would lead one to believe
that the second file name is ".dir-locals-2.el" because of the
dir-locals-file-2 constant; it's actually derived from dir-locals-file only.

So maybe we need to add that to the manual.
>

I have tried to clarify this in the attached patch.

But its (constant) value does override, doesn't it?
>

".dir-locals-2.el" file does override ".dir-locals.el". But that would
happen even if the value of dir-locals-file-2 were "foo.el" i.e. at present
dir-locals-file-2 value is completely unused.. it's just a documentation
holder. I have now moved that documentation to the doc string of
dir-locals-file constant.

Which change?  You are arguing about multiple issues, and I'm confused
> about what change do you have in mind, exactly.
>

Apologies for the confusion, looking at NEWS, I learned about
dir-locals-file-2 constant, but then grepping for that variable led me
nowhere. So I initially thought that that constant was a relic of
now-removed feature. I was wrong about that part.. the feature is still
there and just that dir-locals-file-2 was not used. So the "change" which I
propose in the attached patch is remove dir-locals-file-2 constant and put
the documentation about this feature in the right places.

> That variable is anyways not used.. so what's the point of keeping
> docstring of an unused
> > variable.
>
> The doc string documents a feature.  If you remove the doc string and
> do nothing else, where will that feature be documented?
>

Yes, there was miscommunication.. I proposed to remove that doc string and
add references to ".dir-locals-2.el" wherever applicable (
http://lists.gnu.org/r/emacs-devel/2017-11/msg00568.html).

Then let's fix the documentation.
> I didn't say it was a problem, you misunderstood what I said.  Feel
> free to submit such a patch, and thanks in advance.
>

Thanks! I have attached the patch.
-- 

Kaushal Modi

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

[-- Attachment #2: 0001-Update-documentation-about-.dir-locals-2.el.patch --]
[-- Type: application/octet-stream, Size: 4755 bytes --]

From c7143e7017b732e93d4074da970cd2ef45791362 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Mon, 27 Nov 2017 11:46:54 -0500
Subject: [PATCH] Update documentation about .dir-locals-2.el

* lisp/files.el: Remove unused constant `dir-locals-file-2'.
* lisp/files.el(dir-locals-file):
* doc/lispref/variables.texi (Directory Local Variables): Mention
  ".dir-locals-2.el".

(https://lists.gnu.org/r/emacs-devel/2017-11/msg00649.html)
---
 doc/lispref/variables.texi | 29 +++++++++++++++++------------
 etc/NEWS                   |  2 +-
 lisp/files.el              | 18 +++++++++---------
 3 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi
index a871352b00..c14a440cbf 100644
--- a/doc/lispref/variables.texi
+++ b/doc/lispref/variables.texi
@@ -1911,18 +1911,23 @@ Directory Local Variables
 @defvr Constant dir-locals-file
 This constant is the name of the file where Emacs expects to find the
 directory-local variables.  The name of the file is
-@file{.dir-locals.el}@footnote{
-The MS-DOS version of Emacs uses @file{_dir-locals.el} instead, due to
-limitations of the DOS filesystems.
-}.  A file by that name in a directory causes Emacs to apply its
-settings to any file in that directory or any of its subdirectories
-(optionally, you can exclude subdirectories; see below).
-If some of the subdirectories have their own @file{.dir-locals.el}
-files, Emacs uses the settings from the deepest file it finds starting
-from the file's directory and moving up the directory tree.  The file
-specifies local variables as a specially formatted list; see
-@ref{Directory Variables, , Per-directory Local Variables, emacs, The
-GNU Emacs Manual}, for more details.
+@file{.dir-locals.el}@footnote{ The MS-DOS version of Emacs uses
+@file{_dir-locals.el} instead, due to limitations of the DOS
+filesystems.  }.  A file by that name in a directory causes Emacs to
+apply its settings to any file in that directory or any of its
+subdirectories (optionally, you can exclude subdirectories; see
+below).  If some of the subdirectories have their own
+@file{.dir-locals.el} files, Emacs uses the settings from the deepest
+file it finds starting from the file's directory and moving up the
+directory tree.  This constant is also used to derive the name of a
+second dir-locals file @file{.dir-locals-2.el}.  If this second
+dir-locals file is present, then that is loaded instead of
+@file{.dir-locals.el}.  This is useful when @file{.dir-locals.el} is
+under version control in a shared repository and cannot be used for
+personal customizations.  The file specifies local variables as a
+specially formatted list; see @ref{Directory Variables, ,
+Per-directory Local Variables, emacs, The GNU Emacs Manual}, for more
+details.
 @end defvr
 
 @defun hack-dir-local-variables
diff --git a/etc/NEWS b/etc/NEWS
index f7a9feb6e4..4ccf468693 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -366,7 +366,7 @@ These local variables will thus not vanish on setting a major mode.
 
 +++
 ** A second dir-local file (.dir-locals-2.el) is now accepted.
-See the variable 'dir-locals-file-2' for more information.
+See the doc string of 'dir-locals-file' for more information.
 
 +++
 ** Connection-local variables can be used to specify local variables
diff --git a/lisp/files.el b/lisp/files.el
index d8b38a9f16..16fa4b8c52 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3905,16 +3905,16 @@ dir-locals-file
   "File that contains directory-local variables.
 It has to be constant to enforce uniform values across different
 environments and users.
-See also `dir-locals-file-2', whose values override this one's.
-See Info node `(elisp)Directory Local Variables' for details.")
 
-(defconst dir-locals-file-2 ".dir-locals-2.el"
-  "File that contains directory-local variables.
-This essentially a second file that can be used like
-`dir-locals-file', so that users can have specify their personal
-dir-local variables even if the current directory already has a
-`dir-locals-file' that is shared with other users (such as in a
-git repository).
+A second dir-locals file can be used by a user to specify their
+personal dir-local variables even if the current directory
+already has a `dir-locals-file' that is shared with other
+users (such as in a git repository).  This second file name is
+derived by appending \"-2\" to the file name component without
+extension in `dir-locals-file'.  For example, if the value of
+`dir-locals-file' is \".dir-locals.el\", a \".dir-locals-2.el\"
+file in the same directory will override the \".dir-locals.el\".
+
 See Info node `(elisp)Directory Local Variables' for details.")
 
 (defun dir-locals--all-files (directory)
-- 
2.15.0

Re: Relics of removed dir-locals-file-2 feature in pretest

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Mon, 27 Nov 2017 17:04:21 +0000
> Cc: emacs-devel@gnu.org
>
> Thanks! I have attached the patch. 

Thanks, I have a few comments.

> * lisp/files.el: Remove unused constant `dir-locals-file-2'.

This should be formatted as follows:

 * lisp/files.el (dir-locals-file-2): Remove unused constant.

> * lisp/files.el(dir-locals-file):
                ^^
Missing space.

> * doc/lispref/variables.texi (Directory Local Variables): Mention
>   ".dir-locals-2.el".

These two don't really live together well: one is code, the other
documentation.  So the first one is better written as a separate
entry:

 * lisp/files.el (dir-locals-file): Mention '.dir-locals-2.el' in the
   doc string.

>  doc/lispref/variables.texi | 29 +++++++++++++++++------------
>  etc/NEWS                   |  2 +-
>  lisp/files.el              | 18 +++++++++---------
>  3 files changed, 27 insertions(+), 22 deletions(-)

The change in NEWS should also be mentioned in the log message.

> diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi
> index a871352b00..c14a440cbf 100644
> --- a/doc/lispref/variables.texi
> +++ b/doc/lispref/variables.texi
> @@ -1911,18 +1911,23 @@ Directory Local Variables
>  @defvr Constant dir-locals-file
>  This constant is the name of the file where Emacs expects to find the
>  directory-local variables.  The name of the file is
> -@file{.dir-locals.el}@footnote{
> -The MS-DOS version of Emacs uses @file{_dir-locals.el} instead, due to
> -limitations of the DOS filesystems.
> -}.  A file by that name in a directory causes Emacs to apply its
> -settings to any file in that directory or any of its subdirectories
> -(optionally, you can exclude subdirectories; see below).
> -If some of the subdirectories have their own @file{.dir-locals.el}
> -files, Emacs uses the settings from the deepest file it finds starting
> -from the file's directory and moving up the directory tree.  The file
> -specifies local variables as a specially formatted list; see
> -@ref{Directory Variables, , Per-directory Local Variables, emacs, The
> -GNU Emacs Manual}, for more details.
> +@file{.dir-locals.el}@footnote{ The MS-DOS version of Emacs uses
> +@file{_dir-locals.el} instead, due to limitations of the DOS
> +filesystems.  }.  A file by that name in a directory causes Emacs to
> +apply its settings to any file in that directory or any of its
> +subdirectories (optionally, you can exclude subdirectories; see
> +below).  If some of the subdirectories have their own
> +@file{.dir-locals.el} files, Emacs uses the settings from the deepest
> +file it finds starting from the file's directory and moving up the
> +directory tree.  This constant is also used to derive the name of a
> +second dir-locals file @file{.dir-locals-2.el}.  If this second
> +dir-locals file is present, then that is loaded instead of
> +@file{.dir-locals.el}.  This is useful when @file{.dir-locals.el} is
> +under version control in a shared repository and cannot be used for
> +personal customizations.  The file specifies local variables as a
> +specially formatted list; see @ref{Directory Variables, ,
> +Per-directory Local Variables, emacs, The GNU Emacs Manual}, for more
> +details.

This looks like a lot of changes, but it actually only changes the
last few lines.  Please try not to refill existing text you don't
modify (unless it's really badly formatted), so that the actual
changes are clearly visible.

> +                                      This second file name is
> +derived by appending \"-2\" to the file name component without
> +extension in `dir-locals-file'.

This sentence is slightly confusing.  Suggest to reword:

  The name of this second file is derived by appending \"-2\" to
  the base name of `dir-locals-file'.

> +                                 For example, if the value of
> +`dir-locals-file' is \".dir-locals.el\", a \".dir-locals-2.el\"
> +file in the same directory will override the \".dir-locals.el\".

And this sentence could be made simpler if you just describe the
default case:

  With the default value of `dir-locals-file', a \".dir-locals-2.el\"
  file in the same directory will override \".dir-locals.el\".

Re: Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

Thanks for the quick feedback. My comments are inline below, followed by
updated patch.

On Mon, Nov 27, 2017 at 12:28 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > * lisp/files.el: Remove unused constant `dir-locals-file-2'.
>
> This should be formatted as follows:
>
>  * lisp/files.el (dir-locals-file-2): Remove unused constant.
>

Now fixed (I wasn't quite sure if I should do * foo (bar): if bar was being
removed).

> * lisp/files.el(dir-locals-file):
>                 ^^
> Missing space.
>

Fixed.

> * doc/lispref/variables.texi (Directory Local Variables): Mention
> >   ".dir-locals-2.el".
>
> These two don't really live together well: one is code, the other
> documentation.  So the first one is better written as a separate
> entry:
>
>  * lisp/files.el (dir-locals-file): Mention '.dir-locals-2.el' in the
>    doc string.
>

Fixed. Will keep the point in mind about keeping code and doc separate.

>  doc/lispref/variables.texi | 29 +++++++++++++++++------------
> >  etc/NEWS                   |  2 +-
> >  lisp/files.el              | 18 +++++++++---------
> >  3 files changed, 27 insertions(+), 22 deletions(-)
>
> The change in NEWS should also be mentioned in the log message.
>

Done.

This looks like a lot of changes, but it actually only changes the
> last few lines.  Please try not to refill existing text you don't
> modify (unless it's really badly formatted), so that the actual
> changes are clearly visible.
>

Understood. I thought that doing M-q was the right thing to do. I have now
manually filled only the edited lines.

> +                                      This second file name is
> > +derived by appending \"-2\" to the file name component without
> > +extension in `dir-locals-file'.
>
> This sentence is slightly confusing.  Suggest to reword:
>
>   The name of this second file is derived by appending \"-2\" to
>   the base name of `dir-locals-file'.
>
> > +                                 For example, if the value of
> > +`dir-locals-file' is \".dir-locals.el\", a \".dir-locals-2.el\"
> > +file in the same directory will override the \".dir-locals.el\".
>
> And this sentence could be made simpler if you just describe the
> default case:
>
>   With the default value of `dir-locals-file', a \".dir-locals-2.el\"
>   file in the same directory will override \".dir-locals.el\".
>

Done.

Updated patch is attached. Thank you.
-- 

Kaushal Modi

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

[-- Attachment #2: 0001-Update-documentation-about-.dir-locals-2.el.patch --]
[-- Type: application/octet-stream, Size: 3680 bytes --]

From 9667149c72ba276293a2412c3d56fe522b7a7019 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Mon, 27 Nov 2017 11:46:54 -0500
Subject: [PATCH] Update documentation about .dir-locals-2.el

* lisp/files.el (dir-locals-file-2): Remove unused constant.
* lisp/files.el (dir-locals-file): Mention ".dir-locals-2.el" in the
  doc string.
* doc/lispref/variables.texi (Directory Local Variables): Mention
  ".dir-locals-2.el".
* etc/NEWS: Replace `dir-locals-file-2' mention with
  `dir-locals-file'.

(https://lists.gnu.org/r/emacs-devel/2017-11/msg00649.html)
---
 doc/lispref/variables.texi |  7 ++++++-
 etc/NEWS                   |  2 +-
 lisp/files.el              | 18 +++++++++---------
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi
index a871352b00..5bee0f9d82 100644
--- a/doc/lispref/variables.texi
+++ b/doc/lispref/variables.texi
@@ -1919,7 +1919,12 @@ Directory Local Variables
 (optionally, you can exclude subdirectories; see below).
 If some of the subdirectories have their own @file{.dir-locals.el}
 files, Emacs uses the settings from the deepest file it finds starting
-from the file's directory and moving up the directory tree.  The file
+from the file's directory and moving up the directory tree.  This
+constant is also used to derive the name of a second dir-locals file
+@file{.dir-locals-2.el}.  If this second dir-locals file is present,
+then that is loaded instead of @file{.dir-locals.el}.  This is useful
+when @file{.dir-locals.el} is under version control in a shared
+repository and cannot be used for personal customizations.  The file
 specifies local variables as a specially formatted list; see
 @ref{Directory Variables, , Per-directory Local Variables, emacs, The
 GNU Emacs Manual}, for more details.
diff --git a/etc/NEWS b/etc/NEWS
index f7a9feb6e4..4ccf468693 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -366,7 +366,7 @@ These local variables will thus not vanish on setting a major mode.
 
 +++
 ** A second dir-local file (.dir-locals-2.el) is now accepted.
-See the variable 'dir-locals-file-2' for more information.
+See the doc string of 'dir-locals-file' for more information.
 
 +++
 ** Connection-local variables can be used to specify local variables
diff --git a/lisp/files.el b/lisp/files.el
index d8b38a9f16..8021e1bbed 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3905,16 +3905,16 @@ dir-locals-file
   "File that contains directory-local variables.
 It has to be constant to enforce uniform values across different
 environments and users.
-See also `dir-locals-file-2', whose values override this one's.
-See Info node `(elisp)Directory Local Variables' for details.")
 
-(defconst dir-locals-file-2 ".dir-locals-2.el"
-  "File that contains directory-local variables.
-This essentially a second file that can be used like
-`dir-locals-file', so that users can have specify their personal
-dir-local variables even if the current directory already has a
-`dir-locals-file' that is shared with other users (such as in a
-git repository).
+A second dir-locals file can be used by a user to specify their
+personal dir-local variables even if the current directory
+already has a `dir-locals-file' that is shared with other
+users (such as in a git repository).  The name of this second
+file is derived by appending \"-2\" to the base name of
+`dir-locals-file'.  With the default value of `dir-locals-file',
+a \".dir-locals-2.el\" file in the same directory will override
+the \".dir-locals.el\".
+
 See Info node `(elisp)Directory Local Variables' for details.")
 
 (defun dir-locals--all-files (directory)
-- 
2.15.0

Re: Relics of removed dir-locals-file-2 feature in pretest

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Mon, 27 Nov 2017 17:50:33 +0000
> Cc: emacs-devel@gnu.org
> 
> Updated patch is attached. Thank you.

Thanks, pushed to the release branch.

Re: Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

On Tue, Nov 28, 2017 at 12:20 PM Eli Zaretskii <eliz@gnu.org> wrote:

> Thanks, pushed to the release branch.
>

Thank you.

I see that the reference link was mentioned as:

  See https://lists.gnu.org/r/emacs-devel/2017-11/msg00649.html for more
details.

When I put that link in the message, I couldn't find any rules on how to do
that in on
http://git.savannah.gnu.org/cgit/emacs.git/tree/CONTRIBUTE?h=emacs-26
or *https://www.gnu.org/prep/standards/html_node/Change-Logs.html
<https://www.gnu.org/prep/standards/html_node/Change-Logs.html>.*

For contributing to the Org repo, the style is to mention links simply
within angle brackets:

    <https://lists.gnu.org/r/emacs-devel/2017-11/msg00649.html>

Example:
http://orgmode.org/cgit.cgi/org-mode.git/commit/?id=d7940ae2a7ca01f01d17ba85e113ad7cc65b10ea

Can the same rule be adopted for the Emacs repo and documented in
/CONTRIBUTE?
-- 

Kaushal Modi

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

Re: Relics of removed dir-locals-file-2 feature in pretest

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Tue, 28 Nov 2017 17:34:19 +0000
> Cc: emacs-devel@gnu.org
> 
> When I put that link in the message, I couldn't find any rules on how to do that in on
> http://git.savannah.gnu.org/cgit/emacs.git/tree/CONTRIBUTE?h=emacs-26 or
> https://www.gnu.org/prep/standards/html_node/Change-Logs.html.
> 
> For contributing to the Org repo, the style is to mention links simply within angle brackets:
> 
>     <https://lists.gnu.org/r/emacs-devel/2017-11/msg00649.html>
> 
> Example:
> http://orgmode.org/cgit.cgi/org-mode.git/commit/?id=d7940ae2a7ca01f01d17ba85e113ad7cc65b10ea
> 
> Can the same rule be adopted for the Emacs repo and documented in /CONTRIBUTE?

Why is that important?  It's a nuisance to remember to add the
brackets, so I'd prefer to avoid that.

Re: Relics of removed dir-locals-file-2 feature in pretest

[Kaushal Modi]

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

On Tue, Nov 28, 2017 at 12:52 PM Eli Zaretskii <eliz@gnu.org> wrote:

> Why is that important?


It just brings in some consistency.. just as there are rules for other
parts in the commit message. It also makes commit messages easily parseable
(if ever needed) for links.


>   It's a nuisance to remember to add the
> brackets, so I'd prefer to avoid that.
>

I have used that style in Org commits many times, and it's kind of become a
second nature, so not very difficult to remember if doing that is added as
a guideline to CONTRIBUTE in the "** Commit messages" section. Also, the
"remembering issue" applies to everything else in that same section too :)

But in any case, I thought of bringing this up as I saw that there were no
guidelines on how to put links in commit messages.
-- 

Kaushal Modi

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

Re: Relics of removed dir-locals-file-2 feature in pretest

[Davis Herring]

> Why is that important?  It's a nuisance to remember to add the
> brackets, so I'd prefer to avoid that.

For reference, RFC 3986 encourages <http://www.example.com> as one of 
several styles for delimiting URIs.

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or 
too sparse, it is because mass-energy conversion has occurred during 
shipping.

Re: Relics of removed dir-locals-file-2 feature in pretest

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Davis Herring <herring@lanl.gov>
> Date: Tue, 28 Nov 2017 13:02:40 -0700
> 
> > Why is that important?  It's a nuisance to remember to add the
> > brackets, so I'd prefer to avoid that.
> 
> For reference, RFC 3986 encourages <http://www.example.com> as one of 
> several styles for delimiting URIs.

It's still a nuisance.  And we have features to follow URL at point
(it's even turned on by default).

My vote is NO.