unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#33400: 26.1; Order changed for overriding “nil” mode in dir-locals
@ 2018-11-15 13:21 Neil Roberts
  2018-11-15 18:01 ` bug#33400: [PATCH] Let dir locals for more specific modes override those from less Neil Roberts
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Neil Roberts @ 2018-11-15 13:21 UTC (permalink / raw)
  To: 33400

There is a project called Piglit with a .dir_locals.el file¹ like this:

((nil . ((indent-tabs-mode . t)))
 (python-mode . ((indent-tabs-mode . nil))))

I think the intention is that most files in the project should use
indent-tabs-mode but Python files should not. This doesn’t seem to work
in 26.1 and the definition for the nil mode overrides the definition for
the Python mode and Emacs enables tab indentation for Python files. If I
swap the order of the two lines then the Python mode overrides the nil
mode and it works as intended. However, this appears to be a change in
26.1. Previously the opposite order gets the right behaviour.

In git master the behaviour is fixed again so that the order shown above
makes it work. I bisected the fix down to 97b7e58c4d34722e8b0. However
it doesn’t look like this is deliberately fixing the bug. It looks like
it’s due to the fact that a call like the following ends up reversing
the order of newvars as a side effect:

  (map-merge-with 'list 'func nil newvars)

That commit just makes it avoid calling map-merge-with when there is
only one file. So presumably the bug still exists in git master when
there are multiple files.

Perhaps you could argue that this isn’t really a bug and that having
multiple values for a directory local variable is just undefined
behaviour. However I think the previous behaviour was quite useful and
it would be nice to maintain it. Or maybe ideally it could even just say
that any more specific mode overrides any less specific mode.

This was discussed on StackExchange here:

https://emacs.stackexchange.com/questions/45998

- Neil

1. https://cgit.freedesktop.org/piglit/tree/.dir-locals.el

In GNU Emacs 26.1 (build 1, x86_64-redhat-linux-gnu, GTK+ Version 3.22.30)
 of 2018-06-26 built on buildhw-10.phx2.fedoraproject.org
Windowing system distributor 'Fedora Project', version 11.0.11906000
System Description:	Fedora release 28 (Twenty Eight)

Configured using:
 'configure --build=x86_64-redhat-linux-gnu
 --host=x86_64-redhat-linux-gnu --program-prefix=
 --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
 --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
 --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
 --libexecdir=/usr/libexec --localstatedir=/var
 --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xft --with-xpm --with-x-toolkit=gtk3
 --with-gpm=no --with-xwidgets --with-modules
 build_alias=x86_64-redhat-linux-gnu host_alias=x86_64-redhat-linux-gnu
 'CFLAGS=-DMAIL_USE_LOCKF -O2 -g -pipe -Wall -Werror=format-security
 -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions
 -fstack-protector-strong -grecord-gcc-switches
 -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic
 -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection'
 LDFLAGS=-Wl,-z,relro
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GSETTINGS NOTIFY ACL
LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES THREADS XWIDGETS LCMS2

Important settings:
  value of $LANG: fr_FR.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: GNUmakefile

Minor modes in effect:
  shell-dirtrack-mode: t
  whitespace-mode: t
  show-paren-mode: t
  diff-auto-refine-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t

Load-path shadows:
None found.

Features:
(shadow emacsbug completion info ibuf-ext ibuffer ibuffer-loaddefs
bug-reference autoconf autoconf-mode notmuch-jump mhtml-mode css-mode
eww flyspell js pp rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid
rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn
nxml-ns nxml-mode nxml-outln nxml-rap sgml-mode nxml-util nxml-enc
xmltok dired-aux smerge-mode conf-mode view ffap tramp-cmds tabify imenu
man make-mode apropos ediff-merg ediff-wind ediff-diff ediff-mult
ediff-help ediff-init ediff-util ediff python files-x tramp-cache
tramp-sh tramp tramp-compat tramp-loaddefs trampver ucs-normalize
sh-script smie executable cmake-mode rx cl-print debug grep cl-extra
eieio-opt speedbar sb-image ezimage dframe find-func doc-view jka-compr
image-mode shell pcomplete find-dired shr-color color shr svg dom
misearch multi-isearch sendmail qp ispell mm-archive sort mail-extr
whitespace server gtags paren notmuch hl-line notmuch-message
notmuch-hello notmuch-tree notmuch-show notmuch-print notmuch-crypto
notmuch-mua notmuch-draft notmuch-maildir-fcc notmuch-address
notmuch-company notmuch-parser notmuch-wash diff-mode coolj
notmuch-query goto-addr icalendar diary-lib diary-loaddefs cal-menu
calendar cal-loaddefs notmuch-tag edmacro kmacro crm notmuch-lib
notmuch-version notmuch-compat gnus-html url-queue help-fns radix-tree
help-mode browse-url url-cache mm-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util
url-parse auth-source cl-seq eieio eieio-core eieio-loaddefs url-vars
gnus-art mm-uu mml2015 mm-view mml-smime smime dig mailcap gnus-sum
gnus-group gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source tls
gnutls utf7 netrc nnoo parse-time gnus-spec gnus-int gnus-range gnus-win
gnus nnheader javadoc-help thingatpt advice git log-edit easy-mmode
message rmc puny dired dired-loaddefs format-spec rfc822 mml mml-sec
password-cache epa derived epg epg-config gnus-util rmail rmail-loaddefs
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader
pcvs-util add-log ewoc cus-edit cus-start cus-load iso-transl
color-theme-subdued color-theme-neil-dark cc-mode cc-fonts cc-guess
cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs warnings
elec-pair compile comint ansi-color ring color-theme easymenu wid-edit
cl clang-rename clang-include-fixer let-alist json map seq byte-opt
bytecomp byte-compile cconv clang-format cl-macs gv xml cl-loaddefs
cl-lib time-date mule-util tooltip eldoc electric uniquify ediff-hook
vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core term/tty-colors frame cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting xwidget-internal
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 3743848 478859)
 (symbols 48 51709 1)
 (miscs 40 98163 24087)
 (strings 32 331746 46857)
 (string-bytes 1 25016091)
 (vectors 16 79464)
 (vector-slots 8 2261743 505600)
 (floats 8 665 2273)
 (intervals 56 503332 20703)
 (buffers 992 695))





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: [PATCH] Let dir locals for more specific modes override those from less
  2018-11-15 13:21 bug#33400: 26.1; Order changed for overriding “nil” mode in dir-locals Neil Roberts
@ 2018-11-15 18:01 ` Neil Roberts
  2019-04-27 18:05   ` Noam Postavsky
  2019-05-01 11:49   ` bug#35522: [PATCH v2] " Neil Roberts
  2018-11-16  9:48 ` bug#33400: 26.1; Order changed for overriding “nil” mode in dir-locals Phil Sainty
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Neil Roberts @ 2018-11-15 18:01 UTC (permalink / raw)
  To: 33400

The list of dir local variables to apply is now sorted by the number
of parent modes of the mode used as the key in the association
list. That way when the variables are applied in order the variables
from more specific modes will override those from less specific modes.

If there are directory entries in the list then they are sorted in
order of name length. The list of modes for that dir is then
recursively sorted with the same mechanism. That way variables tied to
a particular subdirectory override those in in a parent directory.

Previously the behaviour didn’t seem to be well defined anyway and was
dependent on the order they appeared in the file. However this order
was changed in version 26.1 and it probably also depended on the
number of dir-local files that are merged.

Bug#33400
---
 lisp/files.el | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/lisp/files.el b/lisp/files.el
index dbac6f614f..09bee6a5f9 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4093,6 +4093,51 @@ dir-locals-find-file
 (declare-function map-merge-with "map" (type function &rest maps))
 (declare-function map-merge "map" (type &rest maps))
 
+(defun dir-locals-get-sort-score (node)
+  "Return a number used for sorting the definitions of dir locals.
+NODE is assumed to be a cons cell where the car is either a
+string or a symbol representing a mode name.
+
+If it is a mode then the the depth of the mode (ie, how many
+parents that mode has) will be returned.
+
+If it is a string then the length of the string plus 1000 will be
+returned.
+
+Otherwise it returns -1.
+
+That way the value can be used to sort the list such that deeper
+modes will be after the other modes. This will be followed by
+directory entries in order of length. If the entries are all
+applied in order then that means the more specific modes will
+override the values specified by the earlier modes and directory
+variables will override modes."
+  (let ((key (car node)))
+    (cond ((null key) -1)
+          ((symbolp key)
+           (let ((mode key)
+                 (depth 0))
+             (while (setq mode (get mode 'derived-mode-parent))
+               (setq depth (1+ depth)))
+             depth))
+          ((stringp key)
+           (+ 1000 (length key)))
+          (t -2))))
+
+(defun dir-locals-sort-variables (variables)
+  "Sorts VARIABLES so that applying them in order has the right effect.
+The variables are compared by dir-locals-get-sort-score. Directory entries
+are then recursively sorted using the same criteria."
+  (setq variables (sort variables
+                        (lambda (a b)
+                          (< (dir-locals-get-sort-score a)
+                             (dir-locals-get-sort-score b)))))
+  (dolist (n variables)
+    (when (stringp (car n))
+      (setcdr n (dir-locals-sort-variables (cdr n)))))
+
+  variables)
+
 (defun dir-locals-read-from-dir (dir)
   "Load all variables files in DIR and register a new class and instance.
 DIR is the absolute name of a directory which must contain at
@@ -4130,6 +4175,7 @@ dir-locals-read-from-dir
                                     variables
                                     newvars))))))
       (setq success latest))
+    (setq variables (dir-locals-sort-variables variables))
     (dir-locals-set-class-variables class-name variables)
     (dir-locals-set-directory-class dir class-name success)
     class-name))
-- 
2.17.1






^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#33400: 26.1; Order changed for overriding “nil” mode in dir-locals
  2018-11-15 13:21 bug#33400: 26.1; Order changed for overriding “nil” mode in dir-locals Neil Roberts
  2018-11-15 18:01 ` bug#33400: [PATCH] Let dir locals for more specific modes override those from less Neil Roberts
@ 2018-11-16  9:48 ` Phil Sainty
  2019-04-25 17:08 ` bug#33400: BUMP: please merge the fix for this bug Mark Janes
  2019-04-26 17:46 ` bug#33400: Merge with bug#30008? Kévin Le Gouguec
  3 siblings, 0 replies; 16+ messages in thread
From: Phil Sainty @ 2018-11-16  9:48 UTC (permalink / raw)
  To: Neil Roberts, 33400

On 16/11/18 2:21 AM, Neil Roberts wrote:
> Perhaps you could argue that this isn’t really a bug and that having
> multiple values for a directory local variable is just undefined
> behaviour. However I think the previous behaviour was quite useful
> and it would be nice to maintain it. Or maybe ideally it could even
> just say that any more specific mode overrides any less specific mode.

I agree -- it makes intuitive sense for it to work that way, and I
firmly believe that people *do* use it that way already (as per the
example given), as the documentation always presented examples with
the nil case first, and therefore following its example would result
in that intuitive functionality (except in 26.1).

I'd like to see this fixed in 26.2, as I feel it's a very unexpected
change which can break things for users in confusing ways.


-Phil





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: BUMP: please merge the fix for this bug
  2018-11-15 13:21 bug#33400: 26.1; Order changed for overriding “nil” mode in dir-locals Neil Roberts
  2018-11-15 18:01 ` bug#33400: [PATCH] Let dir locals for more specific modes override those from less Neil Roberts
  2018-11-16  9:48 ` bug#33400: 26.1; Order changed for overriding “nil” mode in dir-locals Phil Sainty
@ 2019-04-25 17:08 ` Mark Janes
  2019-04-26 17:46 ` bug#33400: Merge with bug#30008? Kévin Le Gouguec
  3 siblings, 0 replies; 16+ messages in thread
From: Mark Janes @ 2019-04-25 17:08 UTC (permalink / raw)
  To: 33400

The fix for this bug has been available for several months.  Major
distributions like debian have rolled out updates to Emacs which include
this bug, harming the development of large community projects like Mesa.

Can someone take a look and merge Neil's fix?





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: Merge with bug#30008?
  2018-11-15 13:21 bug#33400: 26.1; Order changed for overriding “nil” mode in dir-locals Neil Roberts
                   ` (2 preceding siblings ...)
  2019-04-25 17:08 ` bug#33400: BUMP: please merge the fix for this bug Mark Janes
@ 2019-04-26 17:46 ` Kévin Le Gouguec
  2019-04-27 18:06   ` Noam Postavsky
  3 siblings, 1 reply; 16+ messages in thread
From: Kévin Le Gouguec @ 2019-04-26 17:46 UTC (permalink / raw)
  To: 33400

Hi,

I just saw Mark's bump by chance; this reminds me of a report I made a
year ago (bug#30008), which went unnoticed AFAICT.  Are these two issues
related?  If that's the case, I guess they could be merged.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: [PATCH] Let dir locals for more specific modes override those from less
  2018-11-15 18:01 ` bug#33400: [PATCH] Let dir locals for more specific modes override those from less Neil Roberts
@ 2019-04-27 18:05   ` Noam Postavsky
  2019-04-29  7:45     ` Neil Roberts
                       ` (2 more replies)
  2019-05-01 11:49   ` bug#35522: [PATCH v2] " Neil Roberts
  1 sibling, 3 replies; 16+ messages in thread
From: Noam Postavsky @ 2019-04-27 18:05 UTC (permalink / raw)
  To: Neil Roberts; +Cc: 33400

Neil Roberts <bpeeluk@yahoo.co.uk> writes:

> The list of dir local variables to apply is now sorted by the number
> of parent modes of the mode used as the key in the association
> list. That way when the variables are applied in order the variables
> from more specific modes will override those from less specific modes.
>
> If there are directory entries in the list then they are sorted in
> order of name length. The list of modes for that dir is then
> recursively sorted with the same mechanism. That way variables tied to
> a particular subdirectory override those in in a parent directory.
>
> Previously the behaviour didn’t seem to be well defined anyway and was
> dependent on the order they appeared in the file. However this order
> was changed in version 26.1 and it probably also depended on the
> number of dir-local files that are merged.

This patch looks basically good to me (some minor formatting: sentences
should end in double space, and the commit message misses a ChangeLog
formatted entry), but it exceeds to copyright exemption limit.  Would
you be willing to sign papers?





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: Merge with bug#30008?
  2019-04-26 17:46 ` bug#33400: Merge with bug#30008? Kévin Le Gouguec
@ 2019-04-27 18:06   ` Noam Postavsky
  0 siblings, 0 replies; 16+ messages in thread
From: Noam Postavsky @ 2019-04-27 18:06 UTC (permalink / raw)
  To: Kévin Le Gouguec; +Cc: 33400

merge 33400 30008
quit

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

> Hi,
>
> I just saw Mark's bump by chance; this reminds me of a report I made a
> year ago (bug#30008), which went unnoticed AFAICT.  Are these two issues
> related?  If that's the case, I guess they could be merged.

Yes, and I believe Neil's patch covers both.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: [PATCH] Let dir locals for more specific modes override those from less
  2019-04-27 18:05   ` Noam Postavsky
@ 2019-04-29  7:45     ` Neil Roberts
  2019-04-29 14:41       ` Eli Zaretskii
  2019-04-29 12:58     ` Michael Albinus
  2019-05-08  7:18     ` Neil Roberts
  2 siblings, 1 reply; 16+ messages in thread
From: Neil Roberts @ 2019-04-29  7:45 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 33400

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

Noam Postavsky <npostavs@gmail.com> writes:

> This patch looks basically good to me (some minor formatting: sentences
> should end in double space, and the commit message misses a ChangeLog
> formatted entry), but it exceeds to copyright exemption limit.  Would
> you be willing to sign papers?

Would you be able to point me to where I can find these papers? The
CONTRIBUTE document doesn’t seem to mention them.

Thanks.

Regards,
- Neil

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: [PATCH] Let dir locals for more specific modes override those from less
  2019-04-27 18:05   ` Noam Postavsky
  2019-04-29  7:45     ` Neil Roberts
@ 2019-04-29 12:58     ` Michael Albinus
  2019-05-08  7:18     ` Neil Roberts
  2 siblings, 0 replies; 16+ messages in thread
From: Michael Albinus @ 2019-04-29 12:58 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Neil Roberts, 33400

Noam Postavsky <npostavs@gmail.com> writes:

>> The list of dir local variables to apply is now sorted by the number
>> of parent modes of the mode used as the key in the association
>> list. That way when the variables are applied in order the variables
>> from more specific modes will override those from less specific modes.
>>
>> If there are directory entries in the list then they are sorted in
>> order of name length. The list of modes for that dir is then
>> recursively sorted with the same mechanism. That way variables tied to
>> a particular subdirectory override those in in a parent directory.
>>
>> Previously the behaviour didn’t seem to be well defined anyway and was
>> dependent on the order they appeared in the file. However this order
>> was changed in version 26.1 and it probably also depended on the
>> number of dir-local files that are merged.
>
> This patch looks basically good to me (some minor formatting: sentences
> should end in double space, and the commit message misses a ChangeLog
> formatted entry), but it exceeds to copyright exemption limit.  Would
> you be willing to sign papers?

I also believe the patch is worth to be applied, to the emacs-26
branch. It fixes a regression introduced with Emacs 26.1.

I'd appreciate if this behavior, priority of .dir-locals entries, is
documented in the Emacs manual. Could this be added to the patch?

Thanks, and best regards, Michael.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: [PATCH] Let dir locals for more specific modes override those from less
  2019-04-29  7:45     ` Neil Roberts
@ 2019-04-29 14:41       ` Eli Zaretskii
  0 siblings, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2019-04-29 14:41 UTC (permalink / raw)
  To: Neil Roberts; +Cc: npostavs, 33400

> From: Neil Roberts <bpeeluk@yahoo.co.uk>
> Date: Mon, 29 Apr 2019 09:45:41 +0200
> Cc: 33400@debbugs.gnu.org
> 
> Would you be able to point me to where I can find these papers? The
> CONTRIBUTE document doesn’t seem to mention them.

Form sent off-list.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#35522: [PATCH v2] Let dir locals for more specific modes override those from less
  2018-11-15 18:01 ` bug#33400: [PATCH] Let dir locals for more specific modes override those from less Neil Roberts
  2019-04-27 18:05   ` Noam Postavsky
@ 2019-05-01 11:49   ` Neil Roberts
  1 sibling, 0 replies; 16+ messages in thread
From: Neil Roberts @ 2019-05-01 11:49 UTC (permalink / raw)
  To: 35522

The list of dir local variables to apply is now sorted by the number
of parent modes of the mode used as the key in the association list.
That way when the variables are applied in order the variables from
more specific modes will override those from less specific modes.

If there are directory entries in the list then they are sorted in
order of name length.  The list of modes for that dir is then
recursively sorted with the same mechanism.  That way variables tied
to a particular subdirectory override those in in a parent directory.

Previously the behaviour didn’t seem to be well defined anyway and was
dependent on the order they appeared in the file.  However this order
was changed in version 26.1 and it probably also depended on the
number of dir-local files that are merged.

Bug#33400

* lisp/files.el (dir-locals-get-sort-score, dir-locals-sort-variables,
dir-locals-read-from-dir): Sort the dir locals so that more precise
modes and directory-specific entries have override lesser ones.
* doc/emacs/custom.texi (Directory Variables): Document the priority.
---
 doc/emacs/custom.texi | 22 ++++++++++++++++++++
 lisp/files.el         | 47 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 22e352ef9f..3a85907b45 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -1377,6 +1377,28 @@ Directory Variables
 Finally, it specifies a different @file{ChangeLog} file name for any
 file in the @file{src/imported} subdirectory.
 
+If the @file{.dir-locals.el} file contains multiple different values
+for a variable using different mode names or directories, the values
+will be applied in an order such that the values for more specific
+modes take priority over more generic modes.  Values specified under a
+directory have even more priority.  For example:
+
+@example
+((nil . ((fill-column . 40)))
+ (c-mode . ((fill-column . 50)))
+ (prog-mode . ((fill-column . 60)))
+ ("narrow-files" . ((nil . (fill-column 20)))))
+@end example
+
+Files that use @code{c-mode} also match @code{prog-mode} because the
+former inherits from the latter.  The value used for
+@code{fill-column} in C files will however be @code{50} because the
+mode name is more specific than @code{prog-mode}.  Files using other
+modes inheriting from @code{prog-mode} will use @code{60}.  Any file
+under the directory @file{narrow-files} will use the value @code{20}
+even if they use @code{c-mode} because directory entries have priority
+over mode entries.
+
 You can specify the variables @code{mode}, @code{eval}, and
 @code{unibyte} in your @file{.dir-locals.el}, and they have the same
 meanings as they would have in file local variables.  @code{coding}
diff --git a/lisp/files.el b/lisp/files.el
index c05d70a00e..e104e49472 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4108,6 +4108,52 @@ dir-locals-find-file
 (declare-function map-merge-with "map" (type function &rest maps))
 (declare-function map-merge "map" (type &rest maps))
 
+(defun dir-locals-get-sort-score (node)
+  "Return a number used for sorting the definitions of dir locals.
+NODE is assumed to be a cons cell where the car is either a
+string or a symbol representing a mode name.
+
+If it is a mode then the the depth of the mode (ie, how many
+parents that mode has) will be returned.
+
+If it is a string then the length of the string plus 1000 will be
+returned.
+
+Otherwise it returns -1.
+
+That way the value can be used to sort the list such that deeper
+modes will be after the other modes.  This will be followed by
+directory entries in order of length.  If the entries are all
+applied in order then that means the more specific modes will
+override the values specified by the earlier modes and directory
+variables will override modes."
+  (let ((key (car node)))
+    (cond ((null key) -1)
+          ((symbolp key)
+           (let ((mode key)
+                 (depth 0))
+             (while (setq mode (get mode 'derived-mode-parent))
+               (setq depth (1+ depth)))
+             depth))
+          ((stringp key)
+           (+ 1000 (length key)))
+          (t -2))))
+
+(defun dir-locals-sort-variables (variables)
+  "Sorts VARIABLES so that applying them in order has the right effect.
+The variables are compared by dir-locals-get-sort-score.
+Directory entries are then recursively sorted using the same
+criteria."
+  (setq variables (sort variables
+                        (lambda (a b)
+                          (< (dir-locals-get-sort-score a)
+                             (dir-locals-get-sort-score b)))))
+  (dolist (n variables)
+    (when (stringp (car n))
+      (setcdr n (dir-locals-sort-variables (cdr n)))))
+
+  variables)
+
 (defun dir-locals-read-from-dir (dir)
   "Load all variables files in DIR and register a new class and instance.
 DIR is the absolute name of a directory which must contain at
@@ -4145,6 +4191,7 @@ dir-locals-read-from-dir
                                     variables
                                     newvars))))))
       (setq success latest))
+    (setq variables (dir-locals-sort-variables variables))
     (dir-locals-set-class-variables class-name variables)
     (dir-locals-set-directory-class dir class-name success)
     class-name))
-- 
2.17.2






^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#33400: [PATCH] Let dir locals for more specific modes override those from less
  2019-04-27 18:05   ` Noam Postavsky
  2019-04-29  7:45     ` Neil Roberts
  2019-04-29 12:58     ` Michael Albinus
@ 2019-05-08  7:18     ` Neil Roberts
  2019-05-10  1:30       ` Noam Postavsky
  2 siblings, 1 reply; 16+ messages in thread
From: Neil Roberts @ 2019-05-08  7:18 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 33400

Noam Postavsky <npostavs@gmail.com> writes:

> This patch looks basically good to me (some minor formatting:
> sentences should end in double space, and the commit message misses a
> ChangeLog formatted entry), but it exceeds to copyright exemption
> limit. Would you be willing to sign papers?

Apparently my assignment/disclaimer process is complete. I sent a v2 of
the patch which I think addresses the issues you mentioned and also adds
documentation to the manual. I got confused by the complicated bug
tracking system and it ended up as a new bug (#35522).

Regards,
- Neil





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: [PATCH] Let dir locals for more specific modes override those from less
  2019-05-08  7:18     ` Neil Roberts
@ 2019-05-10  1:30       ` Noam Postavsky
  2019-05-10  6:44         ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Noam Postavsky @ 2019-05-10  1:30 UTC (permalink / raw)
  To: Neil Roberts; +Cc: 33400

Neil Roberts <bpeeluk@yahoo.co.uk> writes:
>
> Apparently my assignment/disclaimer process is complete. I sent a v2 of
> the patch which I think addresses the issues you mentioned and also adds
> documentation to the manual. I got confused by the complicated bug
> tracking system and it ended up as a new bug (#35522).

That's alright.  Can we have this in emacs-26 on the grounds that the
order of evaluation was already changed from 25, or should this go to
master?






^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: [PATCH] Let dir locals for more specific modes override those from less
  2019-05-10  1:30       ` Noam Postavsky
@ 2019-05-10  6:44         ` Eli Zaretskii
  2019-05-10 10:57           ` bug#33400: [PATCH v3] " Neil Roberts
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-05-10  6:44 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: bpeeluk, 33400

> From: Noam Postavsky <npostavs@gmail.com>
> Date: Thu, 09 May 2019 21:30:46 -0400
> Cc: 33400@debbugs.gnu.org
> 
> That's alright.  Can we have this in emacs-26 on the grounds that the
> order of evaluation was already changed from 25, or should this go to
> master?

It's fine with me to push this to emacs-26, but please clean up the
formatting, add the documentation requested by Michael, and I think
the new function should be internal one (i.e. 2 dashes in its name).

Thanks.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#33400: [PATCH v3] Let dir locals for more specific modes override those from less
  2019-05-10  6:44         ` Eli Zaretskii
@ 2019-05-10 10:57           ` Neil Roberts
  2019-05-12 14:03             ` Noam Postavsky
  0 siblings, 1 reply; 16+ messages in thread
From: Neil Roberts @ 2019-05-10 10:57 UTC (permalink / raw)
  To: 33400

The list of dir local variables to apply is now sorted by the number
of parent modes of the mode used as the key in the association list.
That way when the variables are applied in order the variables from
more specific modes will override those from less specific modes.

If there are directory entries in the list then they are sorted in
order of name length.  The list of modes for that dir is then
recursively sorted with the same mechanism.  That way variables tied
to a particular subdirectory override those in in a parent directory.

Previously the behaviour didn’t seem to be well defined anyway and was
dependent on the order they appeared in the file.  However this order
was changed in version 26.1 and it probably also depended on the
number of dir-local files that are merged.

Bug#33400

* lisp/files.el (dir-locals-get-sort-score, dir-locals-sort-variables,
dir-locals-read-from-dir): Sort the dir locals so that more precise
modes and directory-specific entries have override lesser ones.
* doc/emacs/custom.texi (Directory Variables): Document the priority.
---
 doc/emacs/custom.texi | 22 ++++++++++++++++++++
 lisp/files.el         | 47 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 22e352ef9f..3a85907b45 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -1377,6 +1377,28 @@ Directory Variables
 Finally, it specifies a different @file{ChangeLog} file name for any
 file in the @file{src/imported} subdirectory.
 
+If the @file{.dir-locals.el} file contains multiple different values
+for a variable using different mode names or directories, the values
+will be applied in an order such that the values for more specific
+modes take priority over more generic modes.  Values specified under a
+directory have even more priority.  For example:
+
+@example
+((nil . ((fill-column . 40)))
+ (c-mode . ((fill-column . 50)))
+ (prog-mode . ((fill-column . 60)))
+ ("narrow-files" . ((nil . (fill-column 20)))))
+@end example
+
+Files that use @code{c-mode} also match @code{prog-mode} because the
+former inherits from the latter.  The value used for
+@code{fill-column} in C files will however be @code{50} because the
+mode name is more specific than @code{prog-mode}.  Files using other
+modes inheriting from @code{prog-mode} will use @code{60}.  Any file
+under the directory @file{narrow-files} will use the value @code{20}
+even if they use @code{c-mode} because directory entries have priority
+over mode entries.
+
 You can specify the variables @code{mode}, @code{eval}, and
 @code{unibyte} in your @file{.dir-locals.el}, and they have the same
 meanings as they would have in file local variables.  @code{coding}
diff --git a/lisp/files.el b/lisp/files.el
index 8477c227bc..ae52fff730 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4110,6 +4110,52 @@ dir-locals-find-file
 (declare-function map-merge-with "map" (type function &rest maps))
 (declare-function map-merge "map" (type &rest maps))
 
+(defun dir-locals--get-sort-score (node)
+  "Return a number used for sorting the definitions of dir locals.
+NODE is assumed to be a cons cell where the car is either a
+string or a symbol representing a mode name.
+
+If it is a mode then the the depth of the mode (ie, how many
+parents that mode has) will be returned.
+
+If it is a string then the length of the string plus 1000 will be
+returned.
+
+Otherwise it returns -1.
+
+That way the value can be used to sort the list such that deeper
+modes will be after the other modes.  This will be followed by
+directory entries in order of length.  If the entries are all
+applied in order then that means the more specific modes will
+override the values specified by the earlier modes and directory
+variables will override modes."
+  (let ((key (car node)))
+    (cond ((null key) -1)
+          ((symbolp key)
+           (let ((mode key)
+                 (depth 0))
+             (while (setq mode (get mode 'derived-mode-parent))
+               (setq depth (1+ depth)))
+             depth))
+          ((stringp key)
+           (+ 1000 (length key)))
+          (t -2))))
+
+(defun dir-locals--sort-variables (variables)
+  "Sorts VARIABLES so that applying them in order has the right effect.
+The variables are compared by dir-locals--get-sort-score.
+Directory entries are then recursively sorted using the same
+criteria."
+  (setq variables (sort variables
+                        (lambda (a b)
+                          (< (dir-locals--get-sort-score a)
+                             (dir-locals--get-sort-score b)))))
+  (dolist (n variables)
+    (when (stringp (car n))
+      (setcdr n (dir-locals--sort-variables (cdr n)))))
+
+  variables)
+
 (defun dir-locals-read-from-dir (dir)
   "Load all variables files in DIR and register a new class and instance.
 DIR is the absolute name of a directory which must contain at
@@ -4147,6 +4193,7 @@ dir-locals-read-from-dir
                                     variables
                                     newvars))))))
       (setq success latest))
+    (setq variables (dir-locals--sort-variables variables))
     (dir-locals-set-class-variables class-name variables)
     (dir-locals-set-directory-class dir class-name success)
     class-name))
-- 
2.17.2






^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#33400: [PATCH v3] Let dir locals for more specific modes override those from less
  2019-05-10 10:57           ` bug#33400: [PATCH v3] " Neil Roberts
@ 2019-05-12 14:03             ` Noam Postavsky
  0 siblings, 0 replies; 16+ messages in thread
From: Noam Postavsky @ 2019-05-12 14:03 UTC (permalink / raw)
  To: Neil Roberts; +Cc: 33400

tags 33400 fixed
close 33400 26.3
quit

Pushed to emacs-26, with a couple of minor fixes:

> * lisp/files.el (dir-locals-get-sort-score, dir-locals-sort-variables,
> dir-locals-read-from-dir): Sort the dir locals so that more precise

We close the parens at end of line for ChangeLog entries:

* lisp/files.el (dir-locals-get-sort-score, dir-locals-sort-variables)
(dir-locals-read-from-dir): Sort the dir locals so that more precise

> +@example
> +((nil . ((fill-column . 40)))
> + (c-mode . ((fill-column . 50)))
> + (prog-mode . ((fill-column . 60)))
> + ("narrow-files" . ((nil . (fill-column 20)))))
> +@end example

The last line in the example needs to be

+ ("narrow-files" . ((nil . ((fill-column . 20))))))

02bee7860f 2019-05-12T09:59:55-04:00 "Let dir locals for more specific modes override those from less"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=02bee7860f7e650ef13e00fe1a7f9a362e3eb001






^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-05-12 14:03 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-15 13:21 bug#33400: 26.1; Order changed for overriding “nil” mode in dir-locals Neil Roberts
2018-11-15 18:01 ` bug#33400: [PATCH] Let dir locals for more specific modes override those from less Neil Roberts
2019-04-27 18:05   ` Noam Postavsky
2019-04-29  7:45     ` Neil Roberts
2019-04-29 14:41       ` Eli Zaretskii
2019-04-29 12:58     ` Michael Albinus
2019-05-08  7:18     ` Neil Roberts
2019-05-10  1:30       ` Noam Postavsky
2019-05-10  6:44         ` Eli Zaretskii
2019-05-10 10:57           ` bug#33400: [PATCH v3] " Neil Roberts
2019-05-12 14:03             ` Noam Postavsky
2019-05-01 11:49   ` bug#35522: [PATCH v2] " Neil Roberts
2018-11-16  9:48 ` bug#33400: 26.1; Order changed for overriding “nil” mode in dir-locals Phil Sainty
2019-04-25 17:08 ` bug#33400: BUMP: please merge the fix for this bug Mark Janes
2019-04-26 17:46 ` bug#33400: Merge with bug#30008? Kévin Le Gouguec
2019-04-27 18:06   ` Noam Postavsky

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).