Relevance of custom vars

[monnier]

[ This is coming up in the context of integrating the code into Emacs. ]

I wonder how important are the various Custom variables, in order to decide how/if we should support them in the code for Emacs≥30:

• editorconfig-get-properties-function: I wonder if it's still relevant now that editorconfig-core-get-properties-hash works well. Is support for the editorconfig executable still necessary/useful sometimes? The new hook actually would like to know where the settings come from (and if there are several relevant .editorconfig files, we'd ideally want to return the settings grouped according to (and annotated with) which file they come from).

• editorconfig-after-apply-functions: With Emacs-30's new hook, the editorconfig code is not supposed to apply the settings but only to return them (they're later applied by Emacs's file/dir-local variables code). So, we'd need to change this to return a list of (VAR . VAL). But it depends on how much this hook is used and for what (e.g. the example in the docstring of activating linum is inconvenient to turn into a (VAR . VAL), but also linum is pretty much obsolete, and whether or not to use linum in a file seems like an abuse of .editorconfig anyway).

• editorconfig-exclude-modes: Is this ever used? If so, for what?

• editorconfig-exclude-regexps: Same questions. Here at least, the commit history does mention some uses:

  • It used to be used to disable EditorConfig for remote files. With the Emacs-30 hook, this is already handled by Emacs's enable-remote-dir-locals.
  • It's still used to disable EditorConfig for the recentf file (issue Always prompt when exit Emacs with recentf-mode on after update #240), but AFAICT the fix for issue Editorconfig causes 'Revert buffer from file ...' #246 should make this unnecessary.

• editorconfig-override-file-local-variables: We can't really obey this setting if we use the new hook (unless the value is nil 🙂).

• editorconfig-override-dir-local-variables: It would take an ugly hack to obey this setting when we use the new hook. The new hook's behavior wants to use the proximity of the .editorconfig file compared to that of the .dir-locals.el file to decide which setting takes precedence.

[10sr]

editorconfig-get-properties-function: I wonder if it's still relevant now that editorconfig-core-get-properties-hash works well. Is support for the editorconfig executable still necessary/useful sometimes?

Originally, editorconfig-emacs did not have its own "core" implementation and always required an external executable to get values from .editorconfig file. I added the core implementation in emacs-lisp later, and at that point it is natural to make it pluggable.
It might be still useful if, for example, we (editorconfig team) add a new feature to .editorconfig spec and only editorconfig-core-c supports that feature. But... it should be a very rare case and I think it is ok now to delete this.

editorconfig-after-apply-functions
editorconfig-exclude-modes
editorconfig-exclude-regexps

I added these variables just to provide users the ability to configure editorconfig-mode.
(For example, we can specify modes to enable whitespace-mode by configuring whitespace-global-modes)
Currently I do not suppose any specific use case, but users may use these variables for something.

;; But I also think it is ok to remove these variables if they make the implementation very complicated...
;; Especially, it seems users can use the new hack-dir-local-get-variables-functions hook in place of hack-dir-local-get-variables-functions.

editorconfig-override-file-local-variables
editorconfig-override-dir-local-variables

I think It is acceptable to remove these variables and always treat them as nil If it is difficult to replicate the behavior of these variables in the case of t 🙆

[monnier]

It might be still useful if, for example, we (editorconfig team) add a new feature to .editorconfig spec and only editorconfig-core-c supports that feature. But... it should be a very rare case and I think it is ok now to delete this.

I'm planning to submit for inclusion in Emacs a version of the code which supports only a subset of the features of the current package (and using a package-version that's smaller than the one in NonGNU ELPA). This is not ideal since it makes future synchronizations harder, but: - It is much easier to add features than to remove them (back backward compatibility reasons). Even more so in Emacs compared to a third party package. - Some of the features have not yet been adapted to work with the new hooks. - The differences should be considered undesirable and temporary, i.e. we should keep working to reconcile the two code bases. [ There are also a few issues around copyright paperwork (not all contributors to `editorconfig-emacs` have signed that paperwork). The vast majority of the code is covered, but there are still some small holes. If we can't get the authors to sign the corresponding paperwork, it's probably easy to rewrite the code (and some of those holes might be small enough not to need paperwork), but if we want to be included in Emacs-30, there's no time to wait to figure it out, so I'm leaning towards leaving corresponding functionality out, as long as it's not super-important and easy to re-add it later. ] In any case, keeping the two code bases in sync will necessarily be a manual two-way sync, because Git doesn't support automatic two-way syncs between different repositories (and Emacs's build doesn't support things like Git submodules).

I added these variables just to provide users the ability to configure editorconfig-mode. (For example, we can specify modes to enable `whitespace-mode` by configuring `whitespace-global-modes`) Currently I do not suppose any specific use case, but users may use these variables for something.

That makes sense. There are a few wrinkles, tho, such as the fact that `editorconfig-exclude-modes` can't be obeyed for the `charset` property. Also, there are other ways to dull the effect of `editorconfig-mode` (e.g. with the new hooks it obeys things like `enable-dir-local-variables`). And if we want to add more control, maybe we'd want to do it in way that works not only for `.editorconfig` but also for `.dir-locals.el` settings.

;; But I also think it is ok to remove these variables if they make the implementation very complicated...

The implementation is not complicated, but the behavior it provides is a bit messy when using the new hooks.

;; Especially, it seems users can use the new `hack-dir-local-get-variables-functions` hook in place of `hack-dir-local-get-variables-functions`.

I'm sorry, I did not understand what you meant to say here.

> editorconfig-override-file-local-variables > editorconfig-override-dir-local-variables I think It is acceptable to remove these variables and always treat them as nil If it is difficult to replicate the behavior of these variables in the case of t 🙆

OK, thanks.

[10sr]

I'm planning to submit for inclusion in Emacs a version of the code
which supports only a subset of the features of the current package
(and using a package-version that's smaller than the one in NonGNU ELPA).
This is not ideal since it makes future synchronizations harder, but:

• It is much easier to add features than to remove them (back backward
  compatibility reasons). Even more so in Emacs compared to a third
  party package.
• Some of the features have not yet been adapted to work with the
  new hooks.
• The differences should be considered undesirable and temporary,
  i.e. we should keep working to reconcile the two code bases.

I see, sounds good considering the tight schedule 👍

That makes sense. There are a few wrinkles, tho, such as the fact that
editorconfig-exclude-modes can't be obeyed for the charset property.
Also, there are other ways to dull the effect of editorconfig-mode
(e.g. with the new hooks it obeys things like
enable-dir-local-variables). And if we want to add more control,
maybe we'd want to do it in way that works not only for .editorconfig
but also for .dir-locals.el settings.

Thanks! It might worth documenting to README of this repository, too 🙌

;; Especially, it seems users can use the new
hack-dir-local-get-variables-functions hook in place of
hack-dir-local-get-variables-functions.

I'm sorry, I did not understand what you meant to say here.
Sorry, I meant to say hack-dir-local-get-variables-functions can be used instead of editorconfig-after-apply-functions!

[monnier]

[ BTW, I just pushed to Emacs `master` "my" editorconfig code. So it will be available in Emacs-30. It's still not enabled by default, tho. I'm hoping we can do that for Emacs-31, but changing defaults is delicate business: it needs to have no visible effect (slowdowns or unexpected interactions) for the users who don't need the feature, even in weird corner cases. ]

I see, sounds good considering the tight schedule 👍

I opened yesterday another issue (about a glob bug) where I'm beginning to merge the Emacs-side changes back into `editorconfig-emacs`. Expect others after that one. 🙂

Thanks! It might worth documenting to README of this repository, too 🙌

+1

Sorry, I meant to say hack-dir-local-get-variables-functions can be used instead of editorconfig-after-apply-functions!

Right. There's also `hack-local-variables-hook`. But none of those `hack-*` hooks provide the editorconfig hash-table, so it's not completely comparable.