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

use __fish_complete_groups to complete group names for chown #3380

Merged
merged 2 commits into from Sep 16, 2016

Conversation

Projects
None yet
2 participants
@andrew-schulman
Contributor

andrew-schulman commented Sep 15, 2016

Description

chown completion chown currently uses cat /etc/group to fetch the list of group names. In Cygwin there's no /etc/group file any more (user and group names are fetched directly from the OS), so when a user tries to tab-complete the group name they get an error message:

ASchulma@LZ77E1AASCHULMA ~/d/fish> chown ASchulma:cat: /etc/group: No such file or directory

This change fixes that by using getent group by preference to get the group names, and falling back to /etc/group. This is more portable, and it also agrees with the logic used for chgrp completion in __fish_complete_groups.

TODOs:

  • Changes to fish usage are reflected in user documenation/manpages.
  • Tests have been added for regressions fixed
Show outdated Hide outdated share/completions/chown.fish
@@ -9,4 +9,4 @@ complete -c chown -s v -l verbose --description "Output diagnostic for every fil
complete -c chown -s h -l help --description "Display help and exit"
complete -c chown -l version --description "Display version and exit"
complete -c chown --description "Username" -a "(__fish_print_users):"
complete -c chown --description "Username" -a "(echo (commandline -ct)| __fish_sgrep -o '.*:')(cat /etc/group |cut -d : -f 1)"
complete -c chown --description "Username" -a "(echo (commandline -ct)| __fish_sgrep -o '.*:')(if test -x /usr/bin/getent; getent group; else; cat /etc/group; end |cut -d : -f 1)"

This comment has been minimized.

@faho

faho Sep 15, 2016

Member

This might also be in /bin or /sbin or /usr/local/bin or /I'm/weird/so/this/is/my/bin - use command -s getent >/dev/null.

@faho

faho Sep 15, 2016

Member

This might also be in /bin or /sbin or /usr/local/bin or /I'm/weird/so/this/is/my/bin - use command -s getent >/dev/null.

This comment has been minimized.

@faho

faho Sep 15, 2016

Member

Wait.... why doesn't this just use __fish_complete_groups?

@faho

faho Sep 15, 2016

Member

Wait.... why doesn't this just use __fish_complete_groups?

This comment has been minimized.

@andrew-schulman

andrew-schulman Sep 15, 2016

Contributor

__fish_complete_groups uses slightly different logic to pull two fields from /etc/group (cut -d : -f 1,4), and adds a tab at the end. Different enough to not be useful for chown completion.

@andrew-schulman

andrew-schulman Sep 15, 2016

Contributor

__fish_complete_groups uses slightly different logic to pull two fields from /etc/group (cut -d : -f 1,4), and adds a tab at the end. Different enough to not be useful for chown completion.

This comment has been minimized.

@faho

faho Sep 15, 2016

Member

The part after the tab is the description. You could just remove it again with string replace -r '\t.*$' ''.

@faho

faho Sep 15, 2016

Member

The part after the tab is the description. You could just remove it again with string replace -r '\t.*$' ''.

This comment has been minimized.

@faho

faho Sep 15, 2016

Member

Though that description might be helpful here as well.

@faho

faho Sep 15, 2016

Member

Though that description might be helpful here as well.

This comment has been minimized.

@andrew-schulman

andrew-schulman Sep 16, 2016

Contributor

Got it. First time I've dug into completions. Yes, __fish_complete_groups works fine here, I'll revise.

@andrew-schulman

andrew-schulman Sep 16, 2016

Contributor

Got it. First time I've dug into completions. Yes, __fish_complete_groups works fine here, I'll revise.

@faho faho added bug windows labels Sep 15, 2016

@faho faho added this to the fish 2.4.0 milestone Sep 15, 2016

@andrew-schulman andrew-schulman changed the title from for group names in chown completion, use getent if available to use __fish_complete_groups to complete group names for chown Sep 16, 2016

@faho faho merged commit 05b52ea into fish-shell:master Sep 16, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Sep 16, 2016

Member

Merged, thanks!

Member

faho commented Sep 16, 2016

Merged, thanks!

@andrew-schulman andrew-schulman referenced this pull request Sep 16, 2016

Merged

use PATH to find getent in __fish_complete_groups #3383

0 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment