Make the configuration options uniform and compact - #2146
Merged
Conversation
Seven options declared a :type that rejected their own default, so Customize showed them as mismatched and refused to edit them: projectile-keymap-prefix, -project-name, -buffers-filter-function, -dirconfig-comment-prefix, -fd-executable, -git-submodule-command and -max-file-buffer-count all document nil as meaningful. While in there, a few more type slips: the two ignored-modes/-buffers lists hold regexps, projectile-known-projects-file is a file (its two siblings were already typed as file and directory), and projectile-other-file-alist had a bare `alist' though its shape is strict. Counts and durations now all use natnum instead of an even split between natnum and integer, and choice replaces the three stray radios - including the one nested inside projectile-sort-order's choice. The :safe predicates were applied to four of the ignore lists and not the other five, which is arbitrary: they all hold nothing but strings and can only widen or narrow a listing. They now share one predicate rather than three copies of the same lambda, and the keyword order is :group :type :safe :package-version throughout.
Three docstrings closed a symbol reference with a markdown backtick instead of a quote, so Emacs never linkified them, and projectile-enable-caching told you to set it to `'persistent' - a form that renders with a stray quote and isn't what you'd type. projectile-use-git-grep didn't say it only affects projectile-grep, projectile-per-project-compilation-buffer described itself as making "the per-project compilation buffer", and projectile-enable-cmake-presets restated its own name where the convention is "When non-nil". Also a trailing space in the darcs command and three-space indentation on the pijul one.
Six identical booleans - projectile-configure-use-comint-mode and its five siblings - become one projectile-use-comint-mode, taking nil, t, or a list of the phases to make interactive. The lifecycle phase descriptor table already existed to drive this, so the six call sites collapse to one predicate. projectile-per-project-compilation-buffer and its per-command twin compose rather than exclude each other, which took a paragraph of each docstring to explain. They become projectile-compilation-buffer-scope, a list of `project' and/or `command' - which is exactly what the two booleans already meant together. All eight remain as obsolete variables and are still honored, so an existing configuration keeps working and only gets a byte-compiler warning.
projectile-svn-command filters the output of `svn list -R' through `grep -v '$/'' to get rid of the directory entries, which end in a slash. But `$' is only an anchor at the end of a pattern - in the middle it's a literal, so the filter has been looking for a dollar sign followed by a slash, and matching nothing, since 2014. The pattern is `/$'. Directories no longer show up among an svn project's files.
projectile-global-ignore-file-patterns was the one member of a family of nine that didn't say "globally-ignored", and "patterns" meant gitignore globs in every sibling while this one holds Emacs regexps. It becomes projectile-globally-ignored-file-regexps. projectile-cmd-hist-ignoredups was the only triple-abbreviated name in the file; projectile-related-files-fn-function had a "fn-function" where its two siblings are plain "-function"; projectile-auto-discover never said what it discovers, which is easy to misread next to projectile-discover-tasks. The reviewable search/replace has five options, and three of them were named after replace though they govern the search reviewer just as much - their own docstrings said so. projectile-replace-max-matches, -async and -scan-chunk-size move to the projectile-search- prefix that the other two already use. All seven old names remain as obsolete aliases.
projectile-tags-file-name is read exactly once, to seed the default of projectile-globally-ignored-files as this file loads, so setting it afterwards - which is what a defcustom invites - has never done anything. Naming the tags file in the ignore list says the same thing once. projectile-go-project-test-function is the only project type with a detection hook of its own, and it's read when the go type is registered at load time. Re-registering the type with your own predicate is the way every other type is customized. Both become obsolete variables that are still honored where they always were, so nothing changes for a configuration that sets them before Projectile loads - the only place setting them ever worked.
Drop projectile-string-list-p: compat, which Projectile already requires, backports list-of-strings-p to the Emacs 28.1 floor, and its implementation is a single pass rather than two. The two folded options now normalize instead of answering one aspect at a time. projectile-compilation-buffer-scope returns the effective list of aspects, folding in the t shorthand and the obsolete booleans, so its callers use plain memq and stop asking the same question three times in a row - and it takes t for "both", matching the shape of projectile-use-comint-mode, which the folding commit left inconsistent. The compat mapping from phase to obsolete comint variable moves out of projectile--lifecycle-phases and next to the variables it names, so the descriptor table describes what a lifecycle phase is again and the whole shim is one contiguous block to delete in 4.0. projectile-tags-file-name and projectile-go-project-test-function are no longer read at all - reading a variable we just declared obsolete, from inside a defcustom's standard value no less, preserved nothing but the ability to set them before load. They stay as obsolete declarations, so an existing configuration still gets told. Also drops two tests: one asserted that define-obsolete-variable-alias aliases, and one pinned an error that only happens on the deprecated path and would change the day that path goes.
The entries of projectile-globally-ignored-buffers are matched with `string-match-p', so "*scratch*" is a regexp - and as one it means a literal `*' (unescaped at position 0), then "scratc", then any number of `h'. It matched the scratch buffer by accident, and matched "*scratchhh*" as happily. Typing the option as `(repeat regexp)' is what surfaced this: relint checks the defaults of anything declared a regexp, and had two complaints. Escaping the asterisks is the fix.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
A pass over all 128 defcustoms, looking for naming that broke its own scheme,
types that didn't match the values they accept, and options that could be one
instead of several.
Seven options document
nilas meaningful but declared a:typethat rejectsit, so Customize showed them as mismatched and refused to edit them at all. Two
families fold into one option each (the six
*-use-comint-modebooleans, andthe two compilation-buffer ones), and seven options move onto the naming scheme
their siblings already used. Every old name stays as an obsolete alias or
variable, so existing configuration keeps working.
One real bug fell out of it:
projectile-svn-commandfilters directories out ofsvn list -Rwithgrep -v '$/', where the$is a literal rather than ananchor, so it never dropped anything. That's been broken since 2014.
eldev test)M-x checkdocwarnings