bug-gnu-emacs
[Top][All Lists]
Advanced

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

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


From: Stefan Monnier
Subject: bug#70105: 30.0.50; Emacs should support EditorConfig out of the box
Date: Wed, 19 Jun 2024 11:18:24 -0400
User-agent: Gnus/5.13 (Gnus v5.13)

>> +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{...})

They're passed through the same checks, i.e. editorconfig.el translates
them to their ELisp equivalent and then we check the result as if it
came from a `dir-locals.el` file.  Usually, the editorconfig.el code
makes sure those tests pass without prompting the user, tho.

>> +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"?

Your question suggests "conflict" is more clear, so I'll use
that, thanks.

> - What does "nearest" mean; which one is preferred?

The settings come from a `.editorconfig` or `.dir-locals.el` file and
apply to some other file.  "nearest" here is the distance between the
file where the settings were found and where they apply.

E.g. a setting from `/foo/bar/baz/.editorconfig` should take priority
over a setting from `/foo/bar/.dir-locals.el` because it's closer to
`/foo/bar/baz/mydir/myfile.agda`.

Not sure you state it more clearly (short of writing out a long
explanation like I've just done, which doesn't seem warranted).

>> +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?

Yup.

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

I have a pending bug-report for a discussion about that.  Basically,
I think `editorconfig-indent-size-vars` is a crutch because the need is
not specific to EditorConfig: we should harmonize the way the "basic
indentation step size" is set in the various major modes.
Users shouldn't need to guess which variable does that for each and
every mode.  Instead we should have one convention for where that
customization is done which applies uniformly to all modes.  But I'm not
completely sure what the convention should be.  E.g. maybe they should
all obey the same `indent-basic-offset` var (which would thus be set
buffer-locally when you want to affect a single mode), or maybe they
should all use a var whose name can be easily computed from the major
mode's name, ...

So, until we decide what the convention should be, I'd refrain from
adding anything about it to that Texinfo node.

>> +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.

Fair enough.

>>  @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:
>>  
>>  * 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.

Thanks.

>> +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".

OK.

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

Hmm... good catch.

>> +(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?

I never know *exactly* what is trimmed, and when I do it's usually not
*quite* the same so I usually assume that the answer is "probably, but
it would take more time to figure out than it's worth".

> Also, is this defsubst unused?

Oh, indeed, I removed all references to it, so it can be deleted, yay!

>> +(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.

OK, I'll remove it (I fully agree with your goal).  But the upstream
maintainers may like it enough that when I later harmonize the code
between Emacs and upstream that function ends up coming back :-(


        Stefan






reply via email to

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