bug#70105: 30.0.50; Emacs should support EditorConfig out of the box

[Stefan Kangas <stefankangas@gmail.com>]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> OK, I have a first cut at the doc done.
> So, here's what I'm proposing, presented as a single diff.
> Some commit messages still need to be improved (and I just noticed that
> the files' copyright lines also need to be fixed), but other than that,
> I think it's about ready.
>
> Comments?  Objections?

Thanks, LGTM.  I have some comments and questions below.

> diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
> index 6bf4cbe00df..5287a90bb71 100644
> --- a/doc/emacs/custom.texi
> +++ b/doc/emacs/custom.texi
> @@ -1550,6 +1550,41 @@ Directory Variables
>  do not visit a file directly but perform work within a directory, such
>  as Dired buffers (@pxref{Dired}).
>
> +@node EditorConfig support
> +@subsubsection Per-Directory Variables via EditorConfig
> +@cindex EditorConfig support
> +
> +The EditorConfig standard is an alternative to the @code{.dir-locals.el}
> +files, which can control only a very small number of variables, but
> +has the advantage of being editor-neutral.  Those settings are stored in
> +files named @code{.editorconfig}.


> +If you want Emacs to obey those settings, you need to enable
> +the @code{editorconfig-mode} minor mode.  This is usually all that is
> +needed: when the mode is activated, Emacs will look for @code{.editorconfig}
> +files whenever a file is visited, just as it does for @code{.dir-locals.el}.

Is it ever dangerous to load settings from these files or is it always
safe?  Should that be documented somewhere, e.g.:

    In contrast to @code{.dir-locals.el}, it is always considered safe
    to load settings from @code{.editorconfig} files.  (See @xref{...})

> +When both @code{.editorconfig} and @code{.dir-locals.el} files are
> +encountered, the corresponding settings are combined, and in case there
   ^^^^^^^^^^^ a simpler word might suffice: "found"
> +is overlap, the settings coming from the nearest file take precedence.

I don't think I understand this:
- Does "overlap" mean "conflict"?
- What does "nearest" mean; which one is preferred?

> +The @code{indent_size} setting of the EditorConfig standard does not
> +correspond to a fixed variable in Emacs, but instead needs to set
> +different variables depending on the major mode.  Ideally all major
> +modes should set the corresponding @code{editorconfig-indent-size-vars},
> +but if you use a major mode in which @code{indent_size} does not take
> +effect because the major mode does not yet support it, you can customize
> +the @code{editorconfig-indentation-alist} variable to tell Emacs which
> +variables need to be set in that major mode.

Does this suggest that our in-tree major modes should set that variable,
IOW that we should spread the knowledge to those modes?  That should
also help encourage major mode authors elsewhere to do the same.

Should this be documented also in (info "(elisp) Major Mode Conventions")?

> +Similarly, there are several different ways to ``trim whitespace'' at
                                                  ^^               ^^
> +the end of lines.  When the EditorConfig @code{trim_trailing_whitespace}
> +setting is used, by default @code{editorconfig-mode} simply calls
> +@code{delete-trailing-whitespace} every time you save your file.
> +If you prefer some other behavior, You can customize
                                      ^^^ lowercase
> +@code{editorconfig-trim-whitespaces-mode} to the minor mode of
> +your preference, such as @code{ws-butler-mode}.

Why square quotes around "trim whitespace"?  I think they can be
removed.

>  @node Connection Variables
>  @subsection Per-Connection Local Variables
>  @cindex local variables, for all remote connections
> diff --git a/etc/NEWS b/etc/NEWS
> index b2fdbc4a88f..06742d24afe 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1964,6 +1964,14 @@ The following new XML schemas are now supported:
>  \f
>  * New Modes and Packages in Emacs 30.1
>
> +** New package EditorConfig.
> +This package provides support for the EditorConfig standard that
> +is an editor-neutral way to provide directory local settings.

Suggested:

    This package provides support for the EditorConfig standard, an
    editor-neutral way to provide directory local (project) settings.

> +It is enabled via a new global minor mode 'editorconfig-mode'
> +which makes Emacs obey the '.editorconfig' files.
> +And the package also comes with a new major mode 'editorconfig-conf-mode'
   ^^^
> +to edit those configuration files.

Suggest replacing "And the package also comes with a new major mode"
with "There is also a new major mode".

> new file mode 100644
> index 00000000000..2b4ddd4410f
> --- /dev/null
> +++ b/lisp/editorconfig-conf-mode.el
> @@ -0,0 +1,95 @@
> +;;; editorconfig-conf-mode.el --- Major mode for editing .editorconfig files  -*- lexical-binding: t -*-
> +
> +;; Copyright (C) 2011-2024 EditorConfig Team
> +
> +;; Author: EditorConfig Team <editorconfig@googlegroups.com>
> +
> +;; See
> +;; https://github.com/editorconfig/editorconfig-emacs/graphs/contributors
> +;; or the CONTRIBUTORS file for the list of contributors.
         ^^^^^^^^^^^^^^^^^^^^^

Suggested:

    "the CONTRIBUTORS file in the linked git repository"

> +(defsubst editorconfig-core-handle--string-trim (str)
> +  "Remove leading and trailing whitespaces from STR."
> +  (replace-regexp-in-string "[[:space:]]+\\'"
> +                            ""
> +                            (replace-regexp-in-string "\\`[[:space:]]+"
> +                                                      ""
> +                                                      str)))

Could this be replaced by string-trim?  Also, is this defsubst unused?

> +(defun editorconfig-core-handle--parse-file (conf)
> +  "Parse EditorConfig file CONF.
> +
> +This function returns a `editorconfig-core-handle'.
> +If CONF is not found return nil."
> +  (when (file-readable-p conf)
> +    (with-temp-buffer
> +      ;; NOTE: Use this instead of insert-file-contents-literally to enable
> +      ;; code conversion
> +      (insert-file-contents conf)
> +      (goto-char (point-min))
> +      (let ((sections ())
> +            (top-props nil)
> +
> +            ;; nil when pattern not appeared yet, "" when pattern is empty ("[]")
> +            (pattern nil)
> +            ;; Alist of properties for current PATTERN
> +            (props ())
> +
> +            ;; Current line num
> +            (current-line-number 1))
> +        (while (not (eobp))
> +          (skip-chars-forward " \t\f")
> +          (cond
> +           ((looking-at "\\(?:[#;].*\\)?$")
> +            nil)
> +
> +           ;; Start of section
> +           ((looking-at "\\[\\(.*\\)\\][ \t]*$")
> +            (let ((newpattern (match-string 1)))
> +              (when pattern
> +                (push (make-editorconfig-core-handle-section
> +                       :name pattern
> +                       :props (nreverse props))
> +                      sections))
> +              (setq props nil)
> +              (setq pattern newpattern)))
> +
> +           ((looking-at "\\([^=: \t]+\\)[ \t]*[=:][ \t]*\\(.*?\\)[ \t]*$")
> +            (let ((key (downcase (match-string 1)))
> +                  (value (match-string 2)))
> +              (when (and (< (length key) 51)
> +                         (< (length value) 256))
> +                (if pattern
> +                    (when (< (length pattern) 4097) ;;FIXME: 4097?
> +                      (push `(,key . ,value)
> +                            props))
> +                  (push `(,key . ,value)
> +                        top-props)))))
> +
> +           (t (error "Error while reading config file: %s:%d:\n    %s\n"
> +                     conf current-line-number
> +                     (buffer-substring-no-properties (line-beginning-position)
> +                                                     (line-end-position)))))
> +          (setq current-line-number (1+ current-line-number))
> +          (goto-char (point-min))
> +          (forward-line (1- current-line-number)))
> +        (when pattern
> +          (push (make-editorconfig-core-handle-section
> +                 :name pattern
> +                 :props (nreverse props))
> +                sections))
> +        (make-editorconfig-core-handle
> +         :top-props (nreverse top-props)
> +         :sections (nreverse sections)
> +         :mtime (nth 5 (file-attributes conf))
> +         :path conf)))))
> +
> +(provide 'editorconfig-core-handle)
> +;;; editorconfig-core-handle.el ends here
> diff --git a/lisp/editorconfig-core.el b/lisp/editorconfig-core.el
> new file mode 100644
> index 00000000000..908d5db6f7e
> --- /dev/null
> +++ b/lisp/editorconfig-core.el
> @@ -0,0 +1,156 @@
> +;;; editorconfig-core.el --- EditorConfig Core library in Emacs Lisp  -*- lexical-binding: t -*-

Suggest scratching "in Emacs Lisp" as redundant

> +(defun editorconfig-version (&optional show-version)
> +  "Get EditorConfig version as string.
> +
> +If called interactively or if SHOW-VERSION is non-nil, show the
> +version in the echo area and the messages buffer."
> +  (interactive (list t))
> +  (let ((version-full
> +         (if (fboundp 'package-get-version)
> +             (package-get-version)
> +           (let* ((version
> +                   (with-temp-buffer
> +                     (require 'find-func)
> +                     (declare-function find-library-name "find-func" (library))
> +                     (insert-file-contents (find-library-name "editorconfig"))
> +                     (require 'lisp-mnt)
> +                     (declare-function lm-version "lisp-mnt" nil)
> +                     (lm-version)))
> +                  (pkg (and (eval-and-compile (require 'package nil t))
> +                            (cadr (assq 'editorconfig
> +                                        package-alist))))
> +                  (pkg-version (and pkg (package-version-join
> +                                         (package-desc-version pkg)))))
> +             (if (and pkg-version
> +                      (not (string= version pkg-version)))
> +                 (concat version "-" pkg-version)
> +               version)))))
> +    (when show-version
> +      (message "EditorConfig Emacs v%s" version-full))
> +    version-full))

Can this function be removed and/or obsoleted?  I would like us to
discourage such functions in favor of using standard functions
(e.g. `M-x list-packages' or `C-h p') to find out what version of a
package is installed.