Address linting errors and restore support for Emacs <28 #77
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kisaragi-hiu
changed the title
Despite adding a third party dependency, this actually lowers the version requirement down from Emacs 28. This also removes the questionable "s-blank" function defined outside of this package's namespace convention.
- The first line should be a full sentence, ending with a period. - Despite that, it shouldn't be longer than 80 characters. - The second line should not be indented. It reads weirdly in *Help* otherwise. - Error messages should NOT end with a period.
This package does not appear to depend on dynamic binding. Lexical binding has been a thing since Emacs 24. "Special" variables (those defined with defvar and similar) are still dynamic in lexical binding mode. In general, lexical binding is either a bit faster or has zero impact, and should always be turned on. Details: https://nullprogram.com/blog/2016/12/22/
kisaragi-hiu
force-pushed
the
address-errors
branch
from
1e874b3 to
bb91dd7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The errors in question
Each commit in this PR fixes a class of issues. This PR also turns on
lexical-binding. Ths issues are:7d9054e string-replace was added in Emacs 28, so up until now this package only worked on Emacs 28 or above. This could be fixed by:
I chose to depend on s.el because (a) it's on MELPA as well, (b) this package was already copying s-blank? inline (as
s-blank, with the wrong package prefix), and (c) there is no need to drop Emacs pre-28 here.I replaced the
s-blankfunction previously defined by this package withs-blank?from s.el, andstring-replacewiths-replace.c3ecb78 Package should have a Homepage or URL header. Previously this package was using a nonstandard "Website" header.
8bd143e 2b25d09 Docstrings should:
*Help*pages)Moreover, there is a strong nudge to add docstrings to all functions and variables. There was just one function without a docstring, so I wrote one for it.
The parentheses and indentation style should also be updated to be more conventional for Emacs Lisp, to make it less painful to edit without introducing lots of style noise. That is not in scope of this PR though, to make this PR easier to review.