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

[Kaushal Modi <kaushal.modi@gmail.com>]

[-- 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