Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relevance of custom vars #343

Open
monnier opened this issue Jun 9, 2024 · 4 comments
Open

Relevance of custom vars #343

monnier opened this issue Jun 9, 2024 · 4 comments
Assignees

Comments

@monnier
Copy link
Collaborator

monnier commented Jun 9, 2024

[ 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:

  • 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
Copy link
Member

10sr commented Jun 14, 2024

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
Copy link
Collaborator Author

monnier commented Jun 15, 2024 via email

@10sr
Copy link
Member

10sr commented Jun 21, 2024

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
Copy link
Collaborator Author

monnier commented Jun 21, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants