emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Kaushal Modi
Subject: Re: Relics of removed dir-locals-file-2 feature in pretest
Date: Mon, 27 Nov 2017 17:50:33 +0000

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 <address@hidden> 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: 0001-Update-documentation-about-.dir-locals-2.el.patch
Description: Binary data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]