Package: emacs;
Reported by: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date: Sun, 31 Mar 2024 13:45:01 UTC
Severity: normal
Found in version 30.0.50
Done: Stefan Monnier <monnier <at> iro.umontreal.ca>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Stefan Monnier <monnier <at> iro.umontreal.ca> To: Eli Zaretskii <eliz <at> gnu.org> Cc: Jen-Chieh Shen <jcs090218 <at> gmail.com>, Björn Bidar <bjorn.bidar <at> thaodan.de>, 10sr <8slashes+git <at> gmail.com>, 70105 <at> debbugs.gnu.org Subject: bug#70105: 30.0.50; Emacs should support EditorConfig out of the box Date: Thu, 06 Jun 2024 19:51:37 -0400
[Message part 1 (text/plain, inline)]
I think I have a clear enough idea of how support for Editorconfig
should be integrated into Emacs, in terms of "how to hook it to the
right places". There are still pending questions about how to adapt
the code of the `editorconfig` package, but in the mean time: here's
a first step to make sure Emacs offers the right hooks.
Find below two patch which do that.
The first patch slightly extends `auto-coding-functions` so that it can
use the file name to decide which coding system to use. This is
a fairly simple change: this clearly the right place to hook ourselves
into. I make the change by adding a dynamically scoped
`auto-coding-file-name` variable. The resulting API would be cleaner if
we instead passed the filename as an additional argument, which we could
do easily (even with some amount of backward compatibility), but the
change proposed has the advantage of being 100% backward compatible.
The cleaner API would replace:
(let ((auto-coding-file-name filename))
(funcall (pop funcs) size))
with
(let ((fun (pop funcs)))
(condition-case nil
(funcall fun size filename)
(wrong-number-of-arguments (funcall fun size))))
In the long run the cleaner API is probably preferable, but it does come
with a slight risk of backward incompatibility.
The second patch adds a new hook
`hack-dir-local-get-variables-functions`. This one is not as "obvious":
there are other places we could hook ourselves into. E.g. it's tempting
to hook deeper into the dir-locals machinery so as to reuse its caching
mechanism. In the end I decided not to do that because the dir-locals
machinery is not prepared to accept settings coming from different
levels of the directory tree, so that trying to shoehorn Editorconfig
into it would be a bit messy and/or unreliable.
The downside is that this hook forces its users to take care of their
own caching. But since the current examples all need to do their
caching a bit differently, I think this is OK.
There's one other design choice I made:
`hack-dir-local-get-variables-functions` returns elements of the form
(DIR . ALIST) where DIR is used to obey
`safe-local-variable-directories' as well as to provide a bit more info
to the user when prompting for confirmation in cache of risky settings.
But that does not provide any actual info about the actual source of the
ALIST, so the message we emit when prompting the user can't tell the
user whether the settings come from a `.dir-locals.el` or
something else. This is not a new problem: the current code already
can't tell the user if the settings come from a `.dir-locals.el`, or
a "project class" or a `.dir-locals-2.el`. So, apparently, this is
a problem we have not deemed worthwhile to fix, so I left it
like unsolved.
I'm hoping this can get into Emacs-30.
Comments/objections?
Stefan
[0001-find-auto-coding-Provide-filename-to-auto-coding-fun.patch (text/x-diff, inline)]
From d12c9bc2a4c8383abdf7fa32b67f1ca0379227e3 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date: Tue, 4 Jun 2024 10:58:29 -0400
Subject: [PATCH 1/2] (find-auto-coding): Provide filename to
`auto-coding-functions`
Allow `auto-coding-functions` to know the file name.
Motivated by the needs of Editorconfig support.
* lisp/international/mule.el (auto-coding-file-name): New var.
(find-auto-coding): Let-bind it for `auto-coding-functions`.
Document the expectation that the arg be an absolute file name.
* doc/lispref/nonascii.texi (Default Coding Systems):
Mention `auto-coding-file-name`.
---
doc/lispref/nonascii.texi | 3 +++
etc/NEWS | 5 +++++
lisp/international/mule.el | 17 ++++++++++++++---
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/doc/lispref/nonascii.texi b/doc/lispref/nonascii.texi
index b33082e2b24..1482becb9f5 100644
--- a/doc/lispref/nonascii.texi
+++ b/doc/lispref/nonascii.texi
@@ -1654,6 +1654,9 @@ Default Coding Systems
starting from point. If the function succeeds in determining a coding
system for the file, it should return that coding system. Otherwise,
it should return @code{nil}.
+Each function can also find the name of the file to which
+the buffer's content belong in the variable
+@code{auto-coding-file-name}.
The functions in this list could be called either when the file is
visited and Emacs wants to decode its contents, and/or when the file's
diff --git a/etc/NEWS b/etc/NEWS
index 808cd0562db..52486b7bbe9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2163,6 +2163,11 @@ completion candidate.
* Lisp Changes in Emacs 30.1
++++
+** 'auto-coding-functions' can know the name of the file.
+The functions on this hook take can now find the name of the file to
+which the text belongs by consulting the variable 'auto-coding-file-name'.
+
+++
** New user option 'compilation-safety' to control safety of native code.
It's now possible to control how safe is the code generated by native
diff --git a/lisp/international/mule.el b/lisp/international/mule.el
index a17221e6d21..ed74fdae755 100644
--- a/lisp/international/mule.el
+++ b/lisp/international/mule.el
@@ -1806,6 +1806,9 @@ auto-coding-regexp-alist-lookup
(setq alist (cdr alist)))))
coding-system)))
+(defvar auto-coding-file-name nil
+ "Variable holding the name of the file for `auto-coding-functions'.")
+
;; See the bottom of this file for built-in auto coding functions.
(defcustom auto-coding-functions '(sgml-xml-auto-coding-function
sgml-html-meta-auto-coding-function)
@@ -1820,6 +1823,9 @@ auto-coding-functions
its contents, and when the file's buffer is about to be saved
and Emacs wants to determine how to encode its contents.
+The name of the file is provided to the function via the variable
+`auto-coding-file-name'.
+
If one of these functions succeeds in determining a coding
system, it should return that coding system. Otherwise, it
should return nil.
@@ -1847,13 +1853,17 @@ auto-coding-alist-lookup
coding-system))
(put 'enable-character-translation 'permanent-local t)
-(put 'enable-character-translation 'safe-local-variable 'booleanp)
+(put 'enable-character-translation 'safe-local-variable #'booleanp)
(defun find-auto-coding (filename size)
+ ;; FIXME: Shouldn't we use nil rather than "" to mean that there's no file?
+ ;; FIXME: Clarify what the SOURCE is for in the return value?
"Find a coding system for a file FILENAME of which SIZE bytes follow point.
These bytes should include at least the first 1k of the file
and the last 3k of the file, but the middle may be omitted.
+FILENAME should be an absolute file name
+or \"\" (which means that there is no associated file).
The function checks FILENAME against the variable `auto-coding-alist'.
If FILENAME doesn't match any entries in the variable, it checks the
contents of the current buffer following point against
@@ -1998,7 +2008,8 @@ find-auto-coding
(setq coding-system (ignore-errors
(save-excursion
(goto-char (point-min))
- (funcall (pop funcs) size)))))
+ (let ((auto-coding-file-name filename))
+ (funcall (pop funcs) size))))))
(if coding-system
(cons coding-system 'auto-coding-functions)))))
@@ -2013,7 +2024,7 @@ set-auto-coding
(if (and found (coding-system-p (car found)))
(car found))))
-(setq set-auto-coding-function 'set-auto-coding)
+(setq set-auto-coding-function #'set-auto-coding)
(defun after-insert-file-set-coding (inserted &optional visit)
"Set `buffer-file-coding-system' of current buffer after text is inserted.
--
2.39.2
[0002-hack-dir-local-get-variables-functions-New-hook.patch (text/x-diff, inline)]
From 71ac0af5d0191cdf8ebc56066c7ca58dd93487cf Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date: Tue, 4 Jun 2024 11:00:32 -0400
Subject: [PATCH 2/2] (hack-dir-local-get-variables-functions): New hook
Make it possible to provide more dir-local variables, such as
done by the Editorconfig package.
* lisp/files.el (hack-dir-local--get-variables): Make arg optional.
(hack-dir-local-get-variables-functions): New hook.
(hack-dir-local-variables): Run it instead of calling
`hack-dir-local--get-variables`.
* doc/lispref/variables.texi (Directory Local Variables):
Document the new hook.
---
doc/lispref/variables.texi | 29 +++++++++++++++
etc/NEWS | 4 ++
lisp/files.el | 76 ++++++++++++++++++++++++++++++--------
3 files changed, 93 insertions(+), 16 deletions(-)
diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi
index e05d3bb0f81..c63057e31ea 100644
--- a/doc/lispref/variables.texi
+++ b/doc/lispref/variables.texi
@@ -2277,6 +2277,35 @@ Directory Local Variables
updates this list.
@end defvar
+@defvar hack-dir-local-get-variables-functions
+This special hook holds the functions that gather the directory-local
+variables to use for a given buffer. By default it contains just the
+function that obeys the other settings described in the present section.
+But it can be used to add support for more sources of directory-local
+variables, such as those used by other text editors.
+
+The functions on this hook are called with no argument, in the buffer to
+which we intend to apply the directory-local variables, after the
+buffer's major mode function has been run, so it can use sources of
+information such as @code{major-mode} or @code{buffer-file-name} to find
+the variables that should be applied.
+
+It should return either a cons cell of the form @code{(@var{directory}
+. @var{alist})} or a list of such cons-cells. A @code{nil} return value
+means that it found no directory-local variables. @var{directory}
+should be a string: the name of the directory to which the variables
+apply. @var{alist} is a list of variables together with their values
+that apply to the current buffer, where every element is of the form
+@code{(@var{varname} . @var{value})}.
+
+The various @var{alist} returned by these functions will be combined,
+and in case of conflicts, the settings coming from deeper directories
+will take precedence over those coming from higher directories in the
+directory hierarchy. Finally, since this hook is run every time we visit
+a file it is important to try and keep those functions efficient, which
+will usually require some of caching.
+@end defvar
+
@defvar enable-dir-local-variables
If @code{nil}, directory-local variables are ignored. This variable
may be useful for modes that want to ignore directory-locals while
diff --git a/etc/NEWS b/etc/NEWS
index 52486b7bbe9..14e5dafbadc 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2163,6 +2163,10 @@ completion candidate.
* Lisp Changes in Emacs 30.1
+** New hook 'hack-dir-local-get-variables-functions'.
+This can be used to provide support for other directory-local settings
+beside '.dir-locals.el'.
+
+++
** 'auto-coding-functions' can know the name of the file.
The functions on this hook take can now find the name of the file to
diff --git a/lisp/files.el b/lisp/files.el
index 210cd0fa7ad..a36ac6b1318 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3494,6 +3494,8 @@ set-auto-mode
;; Check for auto-mode-alist entry in dir-locals.
(with-demoted-errors "Directory-local variables error: %s"
;; Note this is a no-op if enable-local-variables is nil.
+ ;; We don't use `hack-dir-local-get-variables-functions' here, because
+ ;; modes are specific to Emacs.
(let* ((mode-alist (cdr (hack-dir-local--get-variables
(lambda (key) (eq key 'auto-mode-alist))))))
(set-auto-mode--apply-alist mode-alist keep-mode-if-same t)))
@@ -4769,7 +4771,7 @@ enable-remote-dir-locals
(defvar hack-dir-local-variables--warned-coding nil)
-(defun hack-dir-local--get-variables (predicate)
+(defun hack-dir-local--get-variables (&optional predicate)
"Read per-directory local variables for the current buffer.
Return a cons of the form (DIR . ALIST), where DIR is the
directory name (maybe nil) and ALIST is an alist of all variables
@@ -4799,6 +4801,16 @@ hack-dir-local--get-variables
(dir-locals-get-class-variables class)
dir-name nil predicate))))))
+(defvar hack-dir-local-get-variables-functions
+ (list #'hack-dir-local--get-variables)
+ "Special hook to compute the set of dir-local variables.
+Every function is called without arguments and should return either
+a cons of the form (DIR . ALIST) or a (possibly empty) list of such conses,
+where ALIST is an alist of (VAR . VAL) settings.
+DIR should be a string (a directory name) and is used to obey
+`safe-local-variable-directories'.
+This hook is run after the major mode has been setup.")
+
(defun hack-dir-local-variables ()
"Read per-directory local variables for the current buffer.
Store the directory-local variables in `dir-local-variables-alist'
@@ -4806,21 +4818,53 @@ hack-dir-local-variables
This does nothing if either `enable-local-variables' or
`enable-dir-local-variables' are nil."
- (let* ((items (hack-dir-local--get-variables nil))
- (dir-name (car items))
- (variables (cdr items)))
- (when variables
- (dolist (elt variables)
- (if (eq (car elt) 'coding)
- (unless hack-dir-local-variables--warned-coding
- (setq hack-dir-local-variables--warned-coding t)
- (display-warning 'files
- "Coding cannot be specified by dir-locals"))
- (unless (memq (car elt) '(eval mode))
- (setq dir-local-variables-alist
- (assq-delete-all (car elt) dir-local-variables-alist)))
- (push elt dir-local-variables-alist)))
- (hack-local-variables-filter variables dir-name))))
+ (let (items)
+ (when (and enable-local-variables
+ enable-dir-local-variables
+ (or enable-remote-dir-locals
+ (not (file-remote-p (or (buffer-file-name)
+ default-directory)))))
+ (run-hook-wrapped 'hack-dir-local-get-variables-functions
+ (lambda (fun)
+ (let ((res (funcall fun)))
+ (cond
+ ((null res))
+ ((consp (car-safe res))
+ (setq items (append res items)))
+ (t (push res items)))))))
+ ;; Sort the entries from nearest dir to furthest dir.
+ (setq items (sort (nreverse items)
+ :key (lambda (x) (length (car-safe x))) :reverse t))
+ ;; Filter out duplicates, preferring the settings from the nearest dir
+ ;; and from the first hook function.
+ (let ((seen nil))
+ (dolist (item items)
+ (when seen ;; Special case seen=nil since it's the most common case.
+ (setcdr item (seq-filter (lambda (vv) (not (memq (car-safe vv) seen)))
+ (cdr item))))
+ (setq seen (nconc (seq-difference (mapcar #'car (cdr item))
+ '(eval mode))
+ seen))))
+ ;; Rather than a loop, maybe we should handle all the dirs
+ ;; "together", e.g. prompting the user only once. But if so, we'd
+ ;; probably want to also merge the prompt for file-local vars,
+ ;; which comes from the call to `hack-local-variables-filter' in
+ ;; `hack-local-variables'.
+ (dolist (item items)
+ (let ((dir-name (car items))
+ (variables (cdr items)))
+ (when variables
+ (dolist (elt variables)
+ (if (eq (car elt) 'coding)
+ (unless hack-dir-local-variables--warned-coding
+ (setq hack-dir-local-variables--warned-coding t)
+ (display-warning 'files
+ "Coding cannot be specified by dir-locals"))
+ (unless (memq (car elt) '(eval mode))
+ (setq dir-local-variables-alist
+ (assq-delete-all (car elt) dir-local-variables-alist)))
+ (push elt dir-local-variables-alist)))
+ (hack-local-variables-filter variables dir-name))))))
(defun hack-dir-local-variables-non-file-buffer ()
"Apply directory-local variables to a non-file buffer.
--
2.39.2
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.