Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve bzr completion. Closes #3661 #3769

Merged
merged 6 commits into from Jan 25, 2017
Merged

Improve bzr completion. Closes #3661 #3769

merged 6 commits into from Jan 25, 2017

Conversation

@cprieto
Copy link
Contributor

@cprieto cprieto commented Jan 24, 2017

Description

  • Add basic completion for bzr commands
  • Include short and log options for common commands
  • Removed not so common commands

Fixes issue #3661

TODOs:

  • Changes to fish usage are reflected in user documenation/manpages.
  • Tests have been added for regressions fixed
 * Add basic completion for bzr commands
 * Include short and log options for common commands
 * Removed not so common commands
function __fish_bzr_needs_command
set cmd (commandline -opc)

if [ (count $cmd) -eq 1 -a $cmd[1] = 'bzr' ]

This comment has been minimized.

@faho

faho Jan 24, 2017
Member

This has a number of issues, and is an idiom I've been trying to remove from our completions. One is that it'll break with aliases, another is that it'll break with any form of combiner or command decoration (something; and bzr, command bzr).

The best I've found until now is to take the commandline, remove the first element (because that cannot be the subcommand) and then see if any of the following is a command we know.

This comment has been minimized.

@cprieto

cprieto Jan 25, 2017
Author Contributor

What about this, introducing an intermediate step where you get the index in the list with the command name, and from there on just process as usual?

function __fish_bzr_using_command
set cmd (commandline -opc)

if [ (count $cmd) -gt 1 ]

This comment has been minimized.

@faho

faho Jan 24, 2017
Member

I prefer if set -q cmd[2]. (Obviously this could be removed if we weren't so annoying when expanding non-existent list elements)

This comment has been minimized.

@faho

faho Jan 24, 2017
Member

Also, this function would be more useful if it took a list - then the next two would just be __fish_bzr_using_command help and __fish_bzr_using_command add mv.

end

# help commands
complete -f -c bzr -n '__fish_bzr_needs_command' -a help --description 'Show help.'

This comment has been minimized.

@faho

faho Jan 24, 2017
Member

We usually don't use a trailing ".". Brevity is important in descriptions because it allows you to fit more on screen.

complete -f -c bzr -n '__fish_bzr_using_command merge' -l show-base -d 'Show base revision text in conflicts.'
complete -f -c bzr -n '__fish_bzr_using_command merge' -l preview -d 'Instead of merging, show a diff of the merge.'
complete -f -c bzr -n '__fish_bzr_using_command merge' -l interactive -s i -d 'Select changes interactively.'
complete -f -c bzr -n '__fish_bzr_using_command merge' -l directory= -s d -d 'Branch to merge into, rather than the one containing the working directory.'

This comment has been minimized.

@faho

faho Jan 24, 2017
Member

Remove the "=". We always allow "--option=argument" and "--option argument" for long options.

complete -f -c bzr -n '__fish_bzr_common_command' -s v -l verbose --description 'Display more information.'

# Commands with dry-run option
complete -f -c bzr -n '_fish_bzr_dry_run_command' -l dry-run --description 'Show what would be done, but don\'t actually do anything.'

This comment has been minimized.

@faho

faho Jan 24, 2017
Member

Just "Show what would be done" is enough.

@faho
Copy link
Member

@faho faho commented Jan 24, 2017

Thanks for this!

Of course, this being a vcs (and those being really fricking complicated), there's about 50 million more things we could add, but this is a worthwhile first pass.

The failing tests are because of a failing master you've based this on, nothing to worry about.

Also, I'd appreciate any changes as additional commits - some people prefer force-pushing, but I like seeing what changed between versions while reviewing. I'll then squash or otherwise massage it when merging.

cprieto added 5 commits Jan 25, 2017
 * We don't need '=' in long options
To avoid issues pointed out in #3769 helper functions included in fish
are used (__fish_use_subcommand and __fish_seen_subcommand).
@faho faho added this to the next-minor milestone Jan 25, 2017
@faho faho merged commit b7d60dc into fish-shell:master Jan 25, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@faho
Copy link
Member

@faho faho commented Jan 25, 2017

Nice work, thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.