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

Various code cleanups #1693

Merged
merged 3 commits into from
Sep 15, 2024
Merged

Various code cleanups #1693

merged 3 commits into from
Sep 15, 2024

Conversation

tomdl89
Copy link
Member

@tomdl89 tomdl89 commented Oct 5, 2022

The original motivation was to fix the misuse of eval in evil-delay by turning it into a macro (name evil-with-delay). But it includes various generic changes such as prefering #' to quote function names and fixing some incorrect uses of ' in docstrings (many warnings remain about this).

The patch also enables lexical-binding in the remaining files. lexical-binding in evil-ex.el has had a tumultuous life, the last commit of which sets it explicitly to nil while stating confusingly in the commit message that it re-enables it. In any case, I still found some changes needed to account for lexical-binding, so there might be more.

Detailed changes below.

  • evil-common.el (evil-unquote): Delete function, not used (luckily: it reeked of a bad hack to work around a misunderstood bug). (evil--with-delay): New helper function.
    (evil-with-delay): New macro to replace evil-delay. (evil-delay): Rewrite using evil-with-delay and mark as obsolete. (evil-signal-at-bob-or-eob): Fix typos in docstring.

  • evil-core.el (window-configurakion-change-hook): Add FIXME. (evil-define-key): Use evil-with-delay.

  • evil-states.el (evil-visual-activate-hook): Use evil-with-delay.

  • evil-commands.el (evil-match): Use pcase since the branch patterns used are those of pcase rather than those of cl-case. (evil-execute-in-normal-state): Use evil-with-delay.

  • evil-digraphs.el (evil-digraphs-table-user): Remove redundant :group arg.

  • evil-ex.el: Enable lexical-binding like the last commit that touched this cookie said that it was doing (even though it didn't). (evil-ex-info-string): Declare.
    (evil-ex-update): Mark end and len as ignored. (evil-ex-init-shell-argument-completion): Mark arg as ignored. (evil-flatten-syntax-tree): Mark char as ignored. (evil-parser): Rename context to evil--context and declare it as dynbound. Remove unused var last.
    Move shared setq result out of some ifs and conds.

  • evil-jumps.el: Remove redundant :group` arguments.

  • evil-macros.el (evil-define-interactive-code): Move shared setq func out of cond. Move the insertion of quote around func to the cond so the quote is not incorrectly added around lambda forms.

  • evil-pkg.el: Remove file. Move its contents to the pseudo headers of evil.el so (M|NonGNU)ELPA can auto-generate this file appropriately.

  • evil.el: Enable lexical-binding. Synchronize metadata with what was in evil-pkg.el.

  • evil-tests.el: Enable lexical-binding. (evil-test-change-state): Move let to obviate the need for setq. Remove unused vars keymap and local-keymap.
    (evil-test-auxiliary-maps): Rename map to evil--map and declare it as dynbound so evil-define-key can access it.
    (evil-test-exclusive-type): Mark third-line as unused. (evil-test-text-object): Mark type arg as unused. (evil-with-both-search-modules): Move macro before its first use. (evil-test-properties): Rename alist to evil--alist and declare it as dynbound so evil-put-property can access it.

  • evil-command-window.el (evil-command-window-draw-prefix): Mark ignored as, well, ignored.

Closes #1685

@axelf4
Copy link
Collaborator

axelf4 commented Jan 11, 2023

@monnier I rebased this onto master and pushed a commit that fixed the tests and added some optional suggestions. Would be great to get this merged. Two comments:

@monnier
Copy link
Contributor

monnier commented Jan 14, 2023 via email

@monnier
Copy link
Contributor

monnier commented Jul 3, 2023

I pushed to the scratch/evil branch of git://git.sv.git.org/emacs/nongnu.git a reworked version of this patch, split into various commits so it's easier to merge it piecemeal. It also refrains from using #' at those places where it would currently end up adding a "spurious" warnings (fixing those warnings is currently rather inconvenient because of the circular dependencies we have between files).
It also adds one important change which is to replace the use of defadvice with advice-add.

@Hi-Angel
Copy link
Contributor

For the readers, the review for this PR per my understanding is happening here #1819

I also stumbled upon this PR because upstream (not yet released) Emacs has deprecated defadvice, so would nice to have this fixed

This file is auto-generated from headers in `evil.el` anyway.

* evil.el: Synchronize metadata with what was in `evil-pkg.el`.
* .gitignore: Add `evil-pkg.el`.
* Makefile (VERSION): Fetch the info from `evil.el`.
(elpa-pkg.el): New rule to (re)generate the file.
(elpa): Use it (not sure if EASK needs it, tho).
This either requires a dependency on the `nadvice` package,
or bumping the minimum Emacs version to 24.4.  I went with
the `nadvice` package, but maybe bumping up to 24.4 would be better.

* evil.el: Require `nadvice`.

* evil-core.el (evil--advices): New var.
(evil-mode): Use it instead of `ad-dis/enable`.
(evil--advice-add): New function.
(set-window-buffer, select-window, toggle-input-method, use-global-map):
* evil-search.el (isearch-message-prefix, isearch-delete-char)
(isearch-lazy-highlight-search):
* evil-integration.el (keyboard-quit, wdired-change-to-dired-mode)
(show-paren-function, quail-show-key, describe-char, ace-jump-done):
Use `(evil--)advice-add` instead of `defadvice`.
(preceding-sexp, pp-last-sexp): Remove old code for when `advice-add`
is not available.

* evil-repeat.el (evil--read-key-sequence-advice): Adapt to use in
`advice-add`.
(read-key-sequence, read-key-sequence-vector): Use `advice-add`.

* evil-keybindings.el (elp-results): Use `advice-add` and move outside
of `eval-after-load`.
* evil-common.el (evil-with-delay): Add comment for last change.
* evil-macros.el (font-lock-add-keywords): Use the font-lock faces
rather than their obsolete variables.

* evil-ex.el (evil-ex-define-argument-type): Make sure the function
arguments can be compiled.
(evil-ex-init-shell-argument-completion): Don't let-bind
`completion-at-point-functions` because hooks aren't just variables.
@axelf4 axelf4 merged commit ea552ef into master Sep 15, 2024
24 checks passed
@axelf4 axelf4 deleted the monnier-cleanups branch September 15, 2024 08:32
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.

Patch to enable lexical-binding and improve evil-delay
4 participants