alter history sub-command handling #3386
Conversation
This deprecates the use of long options for history sub-commands (e.g., `history --delete`) in favor of proper sub-commands (e.g., `history delete`). It also eliminates the short options for those sub-commands. Also change option processing to allow options anywhere on the command line to match how the vast majority of fish builtins handle flags. Fixes #3367
|
@faho, Please review the completion changes. I personally think file name completions should be disabled for |
|
@krader1961 thanks for doing this! Wiping the current completions out and then using -x (required argument, and no files) seemed to work for me:
Not sure if if I don't know how to build it up properly, or if it's worth suggesting looking at what string does. I'm sure @faho has some ideas. |
|
In fish when you hit a completion where nothing brings any columns, and there are no files, it's a treat to encounter; but surprisingly rare. I think autoloading muddies it up a little. |
|
@krader1961 If you're changing -t - I think it may be an opportunity to change the entire option name. I really do not like "with-" here -- particularly if we will be letting people give that an argument with date format strings in the future. The problem is it looks like it's a predicate, and not a output option. (Like maybe If -t is gone - could that be |
| # --save is not completed; it is for internal use | ||
| # We don't include a completion for the "save" subcommand because it should not be used | ||
| # interactively. | ||
| complete -c history -f -n '__fish_history_no_subcommand' -a search \ |
There was a problem hiding this comment.
You'll want __fish_seen_subcommand_from $__fish_history_all_commands here.
There was a problem hiding this comment.
Done (although it needs a not prefix for the logic to be correct). FWIW, I was simply following the pattern used by a couple of other command completions. It would be good for someone to take a few minutes and normalize all the completions to use this idiom.
| not __fish_history_cmd_in_array $__fish_history_all_commands | ||
| end | ||
|
|
||
| complete -c history -l prefix -s p --description "Match items beginning with the string" |
There was a problem hiding this comment.
-n '__fish_seen_subcommand_from search delete'
There was a problem hiding this comment.
Ahh, yes. That gives the effect I wanted but I didn't see how to do it despite having used -n below.
| complete -c history -l clear --description "Clears history file" | ||
| complete -c history -l merge -s m --description "Incorporate history changes from other sessions" | ||
| complete -c history -l exact -s e --description "Match items in the history that are identicial" | ||
| set -g __fish_history_all_commands search delete save merge clear |
There was a problem hiding this comment.
No need for a global if you expand it directly - use double-quotes where you use it here.
There was a problem hiding this comment.
Given the scoping rules it will be global even without the -g. I vaguely recall a discussion about enhancing this so that there is a "file" scope analogous to python's module scope. The other option is to wrap all of this inside
function __fish_history_completions
...
end
__fish_history_completions
functions -e __fish_history_completions
That allows using set -l. The fix seems worse than simply making it a private global var.
There was a problem hiding this comment.
Actually, when you do set -l in the toplevel of a completion script, it doesn't leak. I'm not sure why it doesn't, though.
|
@floam, As for renaming |
|
Closed by squash merge of 76c73aa. I'd normally leave this PR open a few more days but I've got two more changes to the same blocks of code pending review for inclusion in the 2.4.0 release. |
Nope, that was it. |
This deprecates the use of long options for history sub-commands (e.g.,
history --delete) in favor of proper sub-commands (e.g.,history delete). It also eliminates the short options for those sub-commands.Also change option processing to allow options anywhere on the command
line to match how the vast majority of fish builtins handle flags.
Fixes #3367