Allow overriding preferences in CLI arguments. #251
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
There's no reason we'd want multiple copies of the default settings, so the defaults can be easily extracted into a static variable to clean everything up a bit.
Instead, this patch uses Yaml::Null to indicate an empty options set. Doing things this way cuts down on around 50 lines of code by keeping most of the searching logic in `yaml-rust`'s domain.
The `value` function computes the value associated with a given key for a given filename and extension. This was function was liberally to reduce the repeated logic in the code, and *slightly modifies behavior*, though in a backwards-compatible way. Some settings were only customizeable when declared at a global level in the config file, which may now be specified at the extension/filename level. This does not (yet) apply to some of the more complicated settings like `line_length_guide`: see the diff for mor details.
Rather than keeping the theme in a one-off structure entry, index it with the rest of the preferences values in a Yaml tree. This relies on a fork of `yaml-rust` which provides mutable acccess.
Utilise `borrowed_or` in my yaml-rust fork to shorten the logic.
Applies to getters: + line_comment_prefix + syntax_definition_name
This allows for the configuration to specify a line length guide depending on the file type, for instance, 80 characters everywhere, but 72 in .git/COMMIT_EDITMSG. Note that this changes the Preferences public API, so there are a fair few modifications, including to unit tests.
This implements a simple argument parser that isolates 'direct' arguments (paths, etc) from `--key value` options, and treats the argument --help specially to display a usage message. For instance, the invocation `amp --key value src` would launch amp in the directory src with the option key set to value. Options are not currently used. This commit makes it impossible to pass a file beginning with `--` as a CLI argument: it will be assumed to be an option.
This allows specifying, for instance, `amp <file> --line_length_guide 72` to override any value given in settings. This is done through a change in Application's constructor, allowing the provision of overrides. Because of this change, roughly 80-ish tests had to be modified to pass in empty overrides (thus the long diff).
Cargo.lock had some merge conflicts. This manual merge used cargo to regenerate Cargo.lock (`rm Cargo.lock && cargo build`).
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.
This patchset allows starting Amp as, for instance (
amp --line_length_guide 72 /tmp/file). This is particularly useful in cases where an application creates a temporary file with no identifying extension and opens it in a user-configured editor (for instance, as mutt handles composing email).There are substantial changes to the preferences module to support this change; see the earlier commits for everything that changed there. The weirdest thing about the changes is that my fork of yaml-rust is used to make the code a bit nicer (chyh1990/yaml-rust#179), but other than that I think it's all mostly standard stuff. Refactors could probably go a bit further, but I think it's okay as is.
Also note that in resolving merge conflicts, I just regenerated Cargo.lock, so some dependencies are on slightly later versions (no breaking changes afaict, though).