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
base: master
Are you sure you want to change the base?
Various code cleanups #1693
Conversation
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 `if`s and `cond`s. * 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.
Suggested cleanups: * Remove commented dead code, e.g. keymap testing was moved to evil-test-state-keymaps. * Implicitly use current buffer with buffer-file-name. * vi should probably be capitalized as vi.
ba5b322
to
53cc262
Compare
@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 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 `require`s, 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 :-)
|
I pushed to the |
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 |
The original motivation was to fix the misuse of
eval
inevil-delay
by turning it into a macro (nameevil-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
inevil-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 forlexical-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 usingevil-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 ofpcase
rather than those ofcl-case
. (evil-execute-in-normal-state): Useevil-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
andlen
as ignored. (evil-ex-init-shell-argument-completion): Markarg
as ignored. (evil-flatten-syntax-tree): Markchar
as ignored. (evil-parser): Renamecontext
toevil--context
and declare it as dynbound. Remove unused varlast
.Move shared
setq result
out of someif
s andcond
s.evil-jumps.el: Remove redundant :group` arguments.
evil-macros.el (evil-define-interactive-code): Move shared
setq func
out ofcond
. Move the insertion of quote aroundfunc
to thecond
so thequote
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 inevil-pkg.el
.evil-tests.el: Enable
lexical-binding
. (evil-test-change-state): Movelet
to obviate the need forsetq
. Remove unused varskeymap
andlocal-keymap
.(evil-test-auxiliary-maps): Rename
map
toevil--map
and declare it as dynbound soevil-define-key
can access it.(evil-test-exclusive-type): Mark
third-line
as unused. (evil-test-text-object): Marktype
arg as unused. (evil-with-both-search-modules): Move macro before its first use. (evil-test-properties): Renamealist
toevil--alist
and declare it as dynbound soevil-put-property
can access it.evil-command-window.el (evil-command-window-draw-prefix): Mark
ignored
as, well, ignored.Closes #1685