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

Add wpctl completions #10043

Merged
merged 2 commits into from Nov 24, 2023
Merged

Add wpctl completions #10043

merged 2 commits into from Nov 24, 2023

Conversation

Harazi
Copy link
Contributor

@Harazi Harazi commented Oct 2, 2023

Description

Added completions for WirePlumber Control CLI wpctl

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@Harazi Harazi changed the title Add wpctl completions Add wpctl completions Oct 2, 2023
end

function __wpctl_command_shape
set -l shape (string split --no-empty " " -- $argv)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unnecessary - you're already passing these as a list, so you can just use $argv directly.

if string match -- '-*' $arg
set -e command[$i]
else
set i (math $i + 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop can just be

set -l command (string match -v -- '-*' $command)

Copy link
Member

@faho faho Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not that that's great - you typically would want to use argparse here, but it is equivalent to your loop)

complete -c wpctl -n "__fish_seen_subcommand_from set-mute" -s p -l pid -d "Selects all nodes associated to the given PID number"

complete -c wpctl -n __wpctl_command_shape -a "$commands"
complete -c wpctl -n '__wpctl_command_shape "*"' -n "__fish_seen_subcommand_from get-volume inspect set-volume set-mute set-profile" -a "@DEFAULT_AUDIO_SOURCE@" -d "Default Microphone"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a placeholder or the actual argument wpctl expects?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is a valid argument. @DEFAULT_*@ is mapped to the current default nodes

complete -c wpctl -n "__fish_seen_subcommand_from inspect" -s r -l referenced -d "Show objects that are referenced in properties"
complete -c wpctl -n "__fish_seen_subcommand_from inspect" -s a -l associated -d "Show associated objects"
complete -c wpctl -n "__fish_seen_subcommand_from set-volume" -s p -l pid -d "Selects all nodes associated to the given PID number"
complete -c wpctl -n "__fish_seen_subcommand_from set-volume" -s l -l limit -d "Limits the final volume percentage to below this value. (floating point, 1.0 is 100%)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description is a mouthful - "Limit volume to between 0.0 and 1.0".

Copy link
Contributor Author

@Harazi Harazi Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could set it higher than 1.0 for audio amplification, but we could remove the floating point bit, as that is implied from wpctl get-volume output

complete -c wpctl -n "__fish_seen_subcommand_from status" -s n -l name -d "Display device and node names instead of descriptions"
complete -c wpctl -n "__fish_seen_subcommand_from inspect" -s r -l referenced -d "Show objects that are referenced in properties"
complete -c wpctl -n "__fish_seen_subcommand_from inspect" -s a -l associated -d "Show associated objects"
complete -c wpctl -n "__fish_seen_subcommand_from set-volume" -s p -l pid -d "Selects all nodes associated to the given PID number"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"PID" is always a number, no need for "PID number".


function __wpctl_command_shape
set -l shape (string split --no-empty " " -- $argv)
set command (commandline -poc)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set -l, always.

complete -c wpctl -n '__wpctl_command_shape "*"' -n "__fish_seen_subcommand_from inspect set-profile" -a "@DEFAULT_VIDEO_SOURCE@" -d "Default Camera"
complete -c wpctl -n '__wpctl_command_shape "*"' -n "__fish_seen_subcommand_from get-volume inspect set-volume set-mute set-profile" -a "(__wpctl_get_nodes Audio Sources) (__wpctl_get_nodes Audio Sinks)"
complete -c wpctl -n '__wpctl_command_shape "*"' -n "__fish_seen_subcommand_from inspect set-profile" -a "(__wpctl_get_nodes Audio Sources) (__wpctl_get_nodes Audio Sinks) (__wpctl_get_nodes Video Source)"
complete -c wpctl -n '__wpctl_command_shape set-mute "*"' -a 0\n1\ntoggle
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can use spaces - -a '0 1 toggle'. A bit nicer to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally forgot you could have it space separated

set -l commands status get-volume inspect set-default set-volume set-mute set-profile clear-default

function __wpctl_get_nodes -a section -a type
for node in (string match -r " ├─ $type:.*(?:\n │.*)+" "$(string match -r "$section*(?:\n [├│└].*)*" "$(wpctl status)")" | string match -rg '^ │[ \*]{6}([^\[]*)')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty clever, but in a way that tells me there should be a better way to get this information - wpctl status isn't made for parsing.

Case in point: This doesn't work for me. I run __wpctl_get_nodes Audio Sources and it doesn't return anything.

This is because string match -r "$section*(?:\n [├│└].*)*" "$(wpctl status)" only matches the string "Audio".

Instead, I would try it with a while-read-loop like this:

set -l havesection
set -l havetype
wpctl status | while read -l line
    if set -q havesection[1]
        test -z "$line"; and break
        if set -q havetype[1]
            string match -rq '^\s*\\s*$' -- $line; and break
            echo $line
        else
            string match -q "*$type*" -- $line
            and set havetype 1
        end
    else
        string match -q "$section*" -- $line
        and set havesection 1
    end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the write-up

@faho faho merged commit 3c814bf into fish-shell:master Nov 24, 2023
6 of 7 checks passed
@faho
Copy link
Member

faho commented Nov 24, 2023

Thanks, merged!

@zanchey zanchey added this to the fish 3.7.0 milestone Dec 29, 2023
zanchey pushed a commit that referenced this pull request Dec 29, 2023
* Add wpctl completions

* Reviewed comments

(cherry picked from commit 3c814bf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants