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

editorconfig.el: Hook elsewhere for the coding-system #356

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

monnier
Copy link
Collaborator

@monnier monnier commented Aug 16, 2024

Until now, the file's coding system was set via an advice on insert-file-contents, which depends on being called from within the find-file-noselect advice.
While it mostly works, it doesn't account for all the cases (e.g. it fails to be used when doing write-region), is a bit cumbersome, and does not interact well with Emacs's other ways to specify a coding-system, such as auto-coding-alist.

This patch replaces it with an advice on find-auto-coding, so as to integrate better with the rest of Emacs's handling of files' coding systems. An immediate benefit is that we don't need to special case jar and zip files any more because auto-coding-alist does it for us already (and for many more file types like tar, exe, ...).

  • editorconfig.el (editorconfig-merge-coding-systems): Use merge-coding-systems only when merging is actually necessary. (editorconfig-set-coding-system-revert): Adjust accordingly. (editorconfig--filename-codingsystem-hash): Delete variable. (editorconfig--advice-insert-file-contents): Delete function. (editorconfig--advice-find-auto-coding): New function. (editorconfig--advice-find-file-noselect): Don't mess with coding systems any more.
    (editorconfig--getting-coding-system): New var.
    (editorconfig--get-coding-system): New function.
    (editorconfig-mode): Advise find-auto-coding instead of insert-file-contents.
    (editorconfig-version): Use package-get-version when available. (find-library-name, lm-version): Move their declaration to the place where we have a good reason to think they're defined. (editorconfig-exclude-regexps): Remove jar and zip patterns, not needed any more.

Until now, the file's coding system was set via an advice on
`insert-file-contents`, which depends on being called from within
the `find-file-noselect` advice.
While it mostly works, it doesn't account for all the cases (e.g. it
fails to be used when doing `write-region`), is a bit cumbersome, and
does not interact well with Emacs's other ways to specify
a coding-system, such as `auto-coding-alist`.

This patch replaces it with an advice on `find-auto-coding`, so as
to integrate better with the rest of Emacs's handling of files' coding
systems.  An immediate benefit is that we don't need to special
case jar and zip files any more because `auto-coding-alist` does
it for us already (and for many more file types like tar, exe, ...).

* editorconfig.el (editorconfig-merge-coding-systems):
Use `merge-coding-systems` only when merging is actually necessary.
(editorconfig-set-coding-system-revert): Adjust accordingly.
(editorconfig--filename-codingsystem-hash): Delete variable.
(editorconfig--advice-insert-file-contents): Delete function.
(editorconfig--advice-find-auto-coding): New function.
(editorconfig--advice-find-file-noselect): Don't mess with coding
systems any more.
(editorconfig--getting-coding-system): New var.
(editorconfig--get-coding-system): New function.
(editorconfig-mode): Advise `find-auto-coding` instead of
`insert-file-contents`.
(editorconfig-version): Use `package-get-version` when available.
(find-library-name, lm-version): Move their declaration to the place
where we have a good reason to think they're defined.
(editorconfig-exclude-regexps): Remove jar and zip patterns, not
needed any more.
The new way coding-system are applied means that
`buffer-file-coding-system` is not set when opening a non-existing file.
Instead that happens when the file gets saved.  Adjust the test accordingly.

* ert-tests/editorconfig-tests.el (test-charset):
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

Successfully merging this pull request may close these issues.

1 participant