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

Extend completion support #3123

Merged
merged 21 commits into from Jun 10, 2016

Conversation

Projects
None yet
3 participants
@moverest
Contributor

moverest commented Jun 7, 2016

Adds autocompletion support for: alsamixer, godoc, gofmt, mkdir, netctl-auto, poweroff, termite, udisksctl.

I made these for me but I figure there would be useful here. So there you have them if you want.

Edit: also: goimports, golint, gorename, xz, modinfo, lscpu.

  • User documentation changes not needed
  • Release notes updated
Show outdated Hide outdated share/completions/udisksctl.fish
set __fish_mount_opts async\tUse\ asynchronous\ I/O atime\tUpdate\ time\ on\ each\ access auto\tMounted\ with\ -a defaults\tUse\ default\ options dev\tInterpret\ character/block\ special\ devices exec\tPermit\ executables _netdev\tFilesystem\ uses\ network noatime\tDo\ not\ update\ time\ on\ each\ access noauto\tNot\ mounted\ by\ -a nodev\tDo\ not\ interpret\ character/block\ special\ devices noexec\tDo\ not\ permit\ executables nosuid\tIgnore\ suid\ bits nouser\tOnly\ root\ may\ mount remount\tRemount\ read-only\ filesystem ro\tMount\ read-only rw\tMount\ read-write suid\tAllow\ suid\ bits sync\tUse\ synchronous\ I/O dirsync\tUse\ synchronous\ directory\ operations user\tAny\ user\ may\ mount users\tAny\ user\ may\ mount\ and\ unmount
function __fish_complete_blockdevice

This comment has been minimized.

@faho

faho Jun 7, 2016

Member

This function has been split out recently (7af9e1f). No need to redefine it here.

@faho

faho Jun 7, 2016

Member

This function has been split out recently (7af9e1f). No need to redefine it here.

Show outdated Hide outdated share/completions/udisksctl.fish
end
function __fish_print_mounted_blockdevice
cat /proc/mounts | grep "^/.*" | cut -d " " -f 1

This comment has been minimized.

@faho

faho Jun 7, 2016

Member

Useless use of cat. Also string would be nice here.

@faho

faho Jun 7, 2016

Member

Useless use of cat. Also string would be nice here.

This comment has been minimized.

@moverest

moverest Jun 7, 2016

Contributor
grep "^/.*"  /proc/mounts | cut -d " " -f 1`

I must admit that I am new to string. Should I do something like this?

string match -i -r "^/[^ ]*" < /proc/mounts
@moverest

moverest Jun 7, 2016

Contributor
grep "^/.*"  /proc/mounts | cut -d " " -f 1`

I must admit that I am new to string. Should I do something like this?

string match -i -r "^/[^ ]*" < /proc/mounts

This comment has been minimized.

@faho

faho Jun 7, 2016

Member

Basically, yes. Though the "-i" is superfluous and you should test -r /proc/mounts before - you don't want errors here (this would also apply to cat | grep).

Of course the assumption here is that block devices can't have spaces in their names, which I believe holds, and even if it didn't it would be a very unusual configuration. /etc/mtab replaces spaces with '\040', dunno if it's the same here.

@faho

faho Jun 7, 2016

Member

Basically, yes. Though the "-i" is superfluous and you should test -r /proc/mounts before - you don't want errors here (this would also apply to cat | grep).

Of course the assumption here is that block devices can't have spaces in their names, which I believe holds, and even if it didn't it would be a very unusual configuration. /etc/mtab replaces spaces with '\040', dunno if it's the same here.

Show outdated Hide outdated share/completions/netctl-auto.fish
function __fish_print_netctl-auto_profile
for line in (netctl-auto list)
set -l profile (string sub -s 3 $line)

This comment has been minimized.

@faho

faho Jun 7, 2016

Member

Where does the "3" come from? Does it print whitespace before the actual profile? If so, string trim might be nicer.

@faho

faho Jun 7, 2016

Member

Where does the "3" come from? Does it print whitespace before the actual profile? If so, string trim might be nicer.

This comment has been minimized.

@moverest

moverest Jun 7, 2016

Contributor

The output of netctl-auto list is this:

* wlan0-ActiveProfile1
  wlan0-Profile2
! wlan0-DisabledProfile3

So it removes the "* " and " ". I forgot the "! " case which I am adding now. I'm still reading the man page but I don't know another way to do this nicer.

@moverest

moverest Jun 7, 2016

Contributor

The output of netctl-auto list is this:

* wlan0-ActiveProfile1
  wlan0-Profile2
! wlan0-DisabledProfile3

So it removes the "* " and " ". I forgot the "! " case which I am adding now. I'm still reading the man page but I don't know another way to do this nicer.

This comment has been minimized.

@faho

faho Jun 7, 2016

Member

In that case sub is okay.

@faho

faho Jun 7, 2016

Member

In that case sub is okay.

Show outdated Hide outdated share/completions/netctl-auto.fish
function __fish_print_netctl-auto_profile
for line in (netctl-auto list)
set -l profile (string sub -s 3 $line)
if [ (string sub -l 1 $line) = "*" ]

This comment has been minimized.

@faho

faho Jun 7, 2016

Member

if string match -q '\**' -- $line

@faho

faho Jun 7, 2016

Member

if string match -q '\**' -- $line

@faho faho added this to the 2.3.1 milestone Jun 7, 2016

Show outdated Hide outdated share/completions/mkdir.fish
complete -c mkdir -s m -l mode --description 'Set file mode (as in chmod)' -x
complete -c mkdir -s p -l parents --description 'Make parent directories as needed'
complete -c mkdir -s v -l verbose --description 'Print a message for each created directory'
complete -c mkdir -s Z --description 'Set SELinux security context of each created directory to the default type'

This comment has been minimized.

@floam

floam Jun 8, 2016

Member

On OS X or BSD a few of these on a very prominent command like mkdir might be confusing to a user. The full possible usage is just three short options:

     mkdir [-pv] [-m mode] directory_name ...

The manpage:

SYNOPSIS
     mkdir [-pv] [-m mode] directory_name ...

DESCRIPTION
     The mkdir utility creates the directories named as operands, in the order
     specified, using mode rwxrwxrwx (0777) as modified by the current
     umask(2).

     The options are as follows:

     -m mode
             Set the file permission bits of the final created directory to
             the specified mode.  The mode argument can be in any of the for-
             mats specified to the chmod(1) command.  If a symbolic mode is
             specified, the operation characters ``+'' and ``-'' are inter-
             preted relative to an initial mode of ``a=rwx''.

     -p      Create intermediate directories as required.  If this option is
             not specified, the full path prefix of each operand must already
             exist.  On the other hand, with this option specified, no error
             will be reported if a directory given as an operand already
             exists.  Intermediate directories are created with permission
             bits of rwxrwxrwx (0777) as modified by the current umask, plus
             write and search permission for the owner.

     -v      Be verbose when creating directories, listing them as they are
             created.

     The user must have write permission in the parent directory.

DIAGNOSTICS
     The mkdir utility exits 0 on success, and >0 if an error occurs.

SEE ALSO
     rmdir(1)

COMPATIBILITY
     The -v option is non-standard and its use in scripts is not recommended.

STANDARDS
     The mkdir utility is expected to be IEEE Std 1003.2 (``POSIX.2'') compat-
     ible.
@floam

floam Jun 8, 2016

Member

On OS X or BSD a few of these on a very prominent command like mkdir might be confusing to a user. The full possible usage is just three short options:

     mkdir [-pv] [-m mode] directory_name ...

The manpage:

SYNOPSIS
     mkdir [-pv] [-m mode] directory_name ...

DESCRIPTION
     The mkdir utility creates the directories named as operands, in the order
     specified, using mode rwxrwxrwx (0777) as modified by the current
     umask(2).

     The options are as follows:

     -m mode
             Set the file permission bits of the final created directory to
             the specified mode.  The mode argument can be in any of the for-
             mats specified to the chmod(1) command.  If a symbolic mode is
             specified, the operation characters ``+'' and ``-'' are inter-
             preted relative to an initial mode of ``a=rwx''.

     -p      Create intermediate directories as required.  If this option is
             not specified, the full path prefix of each operand must already
             exist.  On the other hand, with this option specified, no error
             will be reported if a directory given as an operand already
             exists.  Intermediate directories are created with permission
             bits of rwxrwxrwx (0777) as modified by the current umask, plus
             write and search permission for the owner.

     -v      Be verbose when creating directories, listing them as they are
             created.

     The user must have write permission in the parent directory.

DIAGNOSTICS
     The mkdir utility exits 0 on success, and >0 if an error occurs.

SEE ALSO
     rmdir(1)

COMPATIBILITY
     The -v option is non-standard and its use in scripts is not recommended.

STANDARDS
     The mkdir utility is expected to be IEEE Std 1003.2 (``POSIX.2'') compat-
     ible.

This comment has been minimized.

@moverest

moverest Jun 8, 2016

Contributor

In this case, I can do something like this:

if test (uname) = "Linux"
    complete -c mkdir -s Z --description 'Set SELinux security context of each created directory to the default type'
    complete -c mkdir -l context --description 'Like -Z' -f
    complete -c mkdir -l version --description 'Output version'
end

Or I can get rid of the file.

@moverest

moverest Jun 8, 2016

Contributor

In this case, I can do something like this:

if test (uname) = "Linux"
    complete -c mkdir -s Z --description 'Set SELinux security context of each created directory to the default type'
    complete -c mkdir -l context --description 'Like -Z' -f
    complete -c mkdir -l version --description 'Output version'
end

Or I can get rid of the file.

This comment has been minimized.

@faho

faho Jun 8, 2016

Member

What has worked well for us is something like

if mkdir --version >/dev/null ^/dev/null
# GNU
else
# BSD/OSX

this will also work if you install GNU tools on OSX, and will also work on Hurd.

I think the only place we actually check uname for completions is killall, but that's a special case (you can't ever invoke it on Solaris).

@faho

faho Jun 8, 2016

Member

What has worked well for us is something like

if mkdir --version >/dev/null ^/dev/null
# GNU
else
# BSD/OSX

this will also work if you install GNU tools on OSX, and will also work on Hurd.

I think the only place we actually check uname for completions is killall, but that's a special case (you can't ever invoke it on Solaris).

This comment has been minimized.

@moverest

moverest Jun 8, 2016

Contributor

Only killall, modprobe and open indeed. I'll use mkdir --version then.

I guess it's okay if I use uname for modinfo completions? I have a few other command completions done.

@moverest

moverest Jun 8, 2016

Contributor

Only killall, modprobe and open indeed. I'll use mkdir --version then.

I guess it's okay if I use uname for modinfo completions? I have a few other command completions done.

This comment has been minimized.

@floam

floam Jun 8, 2016

Member

Yeah, testing for a feature directly is usually the way to go.

I think perhaps further you should also test, somehow, for SELinux on Linux (or, at least, a SELinux-enabled mkdir), and if that doesn't exist, don't show the --context=CTX (-Z CTX) options.

@floam

floam Jun 8, 2016

Member

Yeah, testing for a feature directly is usually the way to go.

I think perhaps further you should also test, somehow, for SELinux on Linux (or, at least, a SELinux-enabled mkdir), and if that doesn't exist, don't show the --context=CTX (-Z CTX) options.

This comment has been minimized.

@floam

floam Jun 8, 2016

Member

If there is no other OS that has a mod info ...

Errr, seems Solaris does:

http://docs.oracle.com/cd/E23824_01/html/821-1462/modinfo-1m.html

But if you can only manage to provide completions relevant to a Linux modinfo, it seems reasonable to check uname.

@floam

floam Jun 8, 2016

Member

If there is no other OS that has a mod info ...

Errr, seems Solaris does:

http://docs.oracle.com/cd/E23824_01/html/821-1462/modinfo-1m.html

But if you can only manage to provide completions relevant to a Linux modinfo, it seems reasonable to check uname.

This comment has been minimized.

@faho

faho Jun 8, 2016

Member

But if you can only manage to provide completions relevant to a Linux modinfo, it seems reasonable to check uname.

It seems like the linux one has --version and the solaris one doesn't - long options started as a GNUism, so other OSs don't have them, and if they do they're most likely compatible with the GNU stuff.

We should really try not to call uname - as I said, hurd will have GNU tools but won't identify as linux.

I think perhaps further you should also test, somehow, for SELinux on Linux (or, at least, a SELinux-enabled mkdir), and if that doesn't exist, don't show the --context=CTX (-Z CTX) options.

It seems like the canonical way to check if SELinux is enabled is sestatus. Since I think mkdir can also add contexts even if selinux is installed but not enabled, we should get away with just checking command -s sestatus.

@faho

faho Jun 8, 2016

Member

But if you can only manage to provide completions relevant to a Linux modinfo, it seems reasonable to check uname.

It seems like the linux one has --version and the solaris one doesn't - long options started as a GNUism, so other OSs don't have them, and if they do they're most likely compatible with the GNU stuff.

We should really try not to call uname - as I said, hurd will have GNU tools but won't identify as linux.

I think perhaps further you should also test, somehow, for SELinux on Linux (or, at least, a SELinux-enabled mkdir), and if that doesn't exist, don't show the --context=CTX (-Z CTX) options.

It seems like the canonical way to check if SELinux is enabled is sestatus. Since I think mkdir can also add contexts even if selinux is installed but not enabled, we should get away with just checking command -s sestatus.

This comment has been minimized.

@floam

floam Jun 8, 2016

Member

It seems like the linux one has --version and the solaris one doesn't - long options started as a GNUism, so other OSs don't have them, and if they do they're most likely compatible with the GNU stuff.

They seem to be more different than that?

http://docs.oracle.com/cd/E23824_01/html/821-1462/modinfo-1m.html

@floam

floam Jun 8, 2016

Member

It seems like the linux one has --version and the solaris one doesn't - long options started as a GNUism, so other OSs don't have them, and if they do they're most likely compatible with the GNU stuff.

They seem to be more different than that?

http://docs.oracle.com/cd/E23824_01/html/821-1462/modinfo-1m.html

This comment has been minimized.

@moverest

moverest Jun 8, 2016

Contributor

Seems like modinfo is behave in a completely different way on Solaris... :-/

If I understand well, I should have for mkdir:

if command -s sestatus
    complete -c mkdir -s Z --description 'Set SELinux security context of each created directory to the default type'
    complete -c mkdir -l context --description 'Like -Z' -f
end

But, --version is only present with GNU tools, so I should also have:

if mkdir --version >/dev/null ^/dev/null
    complete -c mkdir -l version --description 'Output version'
end
@moverest

moverest Jun 8, 2016

Contributor

Seems like modinfo is behave in a completely different way on Solaris... :-/

If I understand well, I should have for mkdir:

if command -s sestatus
    complete -c mkdir -s Z --description 'Set SELinux security context of each created directory to the default type'
    complete -c mkdir -l context --description 'Like -Z' -f
end

But, --version is only present with GNU tools, so I should also have:

if mkdir --version >/dev/null ^/dev/null
    complete -c mkdir -l version --description 'Output version'
end

This comment has been minimized.

@faho

faho Jun 8, 2016

Member

They seem to be more different than that?

Sure, but that doesn't matter here - if we have --version, then we have the GNU version. If we don't, then it's something else (currently it only seems to be a thing on Solaris).

If I understand well, I should have for mkdir:

Yes, but silence commands stdout - >/dev/null.

But, --version is only present with GNU tools, so I should also have:

Nope - the entire completion here is specific to the GNU version. The BSD version just has -m, -p and -v - just those short options, not the long versions.

So it's more like

if mkdir --version >/dev/null ^/dev/null
    complete -c mkdir -l version -d 'Output version'
    complete -c mkdir -s m -l mode --description 'Set file mode (as in chmod)' -x
    # [...]
else
    complete -c mkdir -s m --description 'Set file mode (as in chmod)' -x
    # [...]
end

In other words: Use how it reacts to "--version" as an indicator.

@faho

faho Jun 8, 2016

Member

They seem to be more different than that?

Sure, but that doesn't matter here - if we have --version, then we have the GNU version. If we don't, then it's something else (currently it only seems to be a thing on Solaris).

If I understand well, I should have for mkdir:

Yes, but silence commands stdout - >/dev/null.

But, --version is only present with GNU tools, so I should also have:

Nope - the entire completion here is specific to the GNU version. The BSD version just has -m, -p and -v - just those short options, not the long versions.

So it's more like

if mkdir --version >/dev/null ^/dev/null
    complete -c mkdir -l version -d 'Output version'
    complete -c mkdir -s m -l mode --description 'Set file mode (as in chmod)' -x
    # [...]
else
    complete -c mkdir -s m --description 'Set file mode (as in chmod)' -x
    # [...]
end

In other words: Use how it reacts to "--version" as an indicator.

Show outdated Hide outdated share/completions/modinfo.fish
@@ -0,0 +1,16 @@
function __fish_print_modules

This comment has been minimized.

@faho

faho Jun 8, 2016

Member

This is the same thing as in the modprobe completions. It should be split out into its own file in share/functions.

@faho

faho Jun 8, 2016

Member

This is the same thing as in the modprobe completions. It should be split out into its own file in share/functions.

Show outdated Hide outdated share/completions/mkdir.fish
# Checks if SELinux is installed
if command -s sestatus > /dev/null ^ /dev/null
complete -c mkdir -s Z --description 'Set SELinux security context of each created directory to the default type'
complete -c mkdir -l context --description 'Like -Z' -f

This comment has been minimized.

@floam

floam Jun 8, 2016

Member

I see that -Z is the short option for --context -- so instead of giving it a description of "Like -Z", just let it have the same description and have fish show the options together .

@floam

floam Jun 8, 2016

Member

I see that -Z is the short option for --context -- so instead of giving it a description of "Like -Z", just let it have the same description and have fish show the options together .

moverest added some commits Jun 8, 2016

Fix and enhance netctl-auto completions
I mixed things up with `netctl` somehow. Since the two are quite
different they do not have the same function, they should not have
the same completions.

I also find that I would be smarter to only display the relevent
profiles given what we want to do. If we want to disable a profile
we should only complete with enabled profile for completion for
instance. I don't know if the implemention is nice enough however.

@faho faho merged commit 5d20750 into fish-shell:master Jun 10, 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 Jun 10, 2016

Member

I've merged this now. Since there's a bunch of commits, I feel a merge commit is appropriate here.

Thanks!

Member

faho commented Jun 10, 2016

I've merged this now. Since there's a bunch of commits, I feel a merge commit is appropriate here.

Thanks!

@krader1961 krader1961 referenced this pull request Jun 21, 2016

Closed

Roll a 2.3.1 release #3106

krader1961 added a commit that referenced this pull request Jun 21, 2016

@zanchey zanchey changed the title from Extend autocompletion support to Extend completion support Jul 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment