evil icon indicating copy to clipboard operation
evil copied to clipboard

Various code cleanups

Open tomdl89 opened this issue 3 years ago • 4 comments
trafficstars

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

tomdl89 avatar Oct 05 '22 09:10 tomdl89

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

  • This causes quite a lot of bytecomp warnings in evil-vars.el due to the now sharp-quoted functions.
  • How does this compare to pull request #1419?

axelf4 avatar Jan 11 '23 20:01 axelf4

@monnier I rebased this onto master and pushed a commit that fixed the tests and added some optional suggestions.

Thanks a lot.

  • This causes quite a lot of bytecomp warnings in evil-vars.el due to the now sharp-quoted functions.

Yup. Some of them can probably be addressed with a few requires, but probably not all. Maybe it's best to refrain from using those #' thingies in the key bindings for now.

  • How does this compare to pull request #1419?

It short-circuits the "quote the code in the function and then try to undo the damage in the compiler macro" by using a macro :-)

monnier avatar Jan 14 '23 02:01 monnier

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.

monnier avatar Jul 03 '23 15:07 monnier

For the readers, the review for this PR per my understanding is happening here https://github.com/emacs-evil/evil/issues/1819

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

Hi-Angel avatar Feb 15 '24 03:02 Hi-Angel