main: move 'jump' and 'visit' to subcommands #72
Merged
Conversation
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
This fixes #70. Prior to this change, there was a mix of subcommands and flags that were all mutually-exclusive behaviors. This ended up being a bit awkward and made some configurations ambiguous. This moves to a much nicer structure. There are global flags (right now just debug), and then there are subcommands for each distinct mode of operation. For ease of migration, the previous logic still remains, but it can be safely deleted after another release or so to allow migration.
This also updates the help output test for the previous commit's changes.
Prior to this it was just 'pazi' which did so, but it's more useful in the long run to have 'pazi' by itself print help. This provides a migration path to that. This also fixes a regression that would have occured if all the deprecated code were deleted.
|
After this is merged:
|
This allows properly using it in the 'match' expression.
| @@ -90,13 +136,18 @@ fn _main() -> PaziResult { | |||
| .long("interactive") | |||
| .short("i"), | |||
| ) | |||
| // Deprecated in favor of .PaziSubcommand::Visit | |||
| // left temporarily for backwards compatibility | |||
| // Remove before 1.0 | |||
| .arg( | |||
| Arg::with_name("add-dir") | |||
There was a problem hiding this comment.
Should this be hidden as well?
| .arg( | ||
| Arg::with_name("add-dir") | ||
| .help("add a directory to the frecency index") | ||
| .long("add-dir") | ||
| .takes_value(true) | ||
| .value_name("directory"), | ||
| ) | ||
| // Deprecated per above deprecations |
There was a problem hiding this comment.
Ditto here for dir_target
src/main.rs
Outdated
| Some(to) => { | ||
| env::current_dir() | ||
| .map(|cwd| { | ||
| frecency.maybe_add_relative_to(cwd, to); |
There was a problem hiding this comment.
Do we want this behavior here? It makes sense for jump (to help jumpstart (heh) a small database) but for view I'm not so sure.
There was a problem hiding this comment.
I was just keeping the behavior it had previously since this is a refactor more than anything else.
Now that the code is split out properly, it does make sense to drop that here I think.
There was a problem hiding this comment.
Actually, it doens't do save_to_disk in this function (nor did it before) so that's entirely dead code. Good catch.
It didn't actually save the change before, so this is behaviorally identical, just deletes some pointless code.
LinuxMercedes
approved these changes
Jul 2, 2018
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 fixes #70.
Prior to this change, there was a mix of subcommands and flags that were
all mutually-exclusive behaviours.
This ended up being a bit awkward and made some configurations
ambiguous.
This moves to a much nicer structure. There are global flags (right now
just debug), and then there are subcommands for each distinct
mode of operation.
For ease of migration, the previous logic still remains, but it can be
safely deleted after another release or so to allow migration.