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 completion for bluetoothctl #742

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add completion for bluetoothctl #742

wants to merge 1 commit into from

Conversation

audunmg
Copy link

@audunmg audunmg commented Apr 22, 2022

I've used this for some months now and thought I'd share in the hope that it's useful for someone else.

Please let me know of any changes I should make to make it fit in here. I tried my best, but since it's my first completion i might have missed something.

Copy link
Collaborator

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

Have you checked CONTRIBUTING.md and consulted with the upstream BlueZ about it? The Zsh completion seems to be already in the repository, so I guess they would accept your contribution to the upstream. They seem to accept patches through the developer mailing list.

{
local cur=${COMP_WORDS[COMP_CWORD]}
local prev=${COMP_WORDS[COMP_CWORD-1]}
if [[ "$COMP_CWORD" == "1" ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if [[ "$COMP_CWORD" == "1" ]]; then
if ((COMP_CWORD == 1)); then
  • You can use the arithmetic command.

The same applies to the later lines testing COMP_CWORD.

local prev=${COMP_WORDS[COMP_CWORD-1]}
if [[ "$COMP_CWORD" == "1" ]]; then
# Parse help for list of commands and remove ansi colors
COMPREPLY=( $( compgen -W "$(bluetoothctl help | sed -n '/ /{s/\x1B\[[0-9;]*[a-zA-Z]//g;s/\x01//g;s/\x02//g;s/ .*//p}')" -- $cur))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
COMPREPLY=( $( compgen -W "$(bluetoothctl help | sed -n '/ /{s/\x1B\[[0-9;]*[a-zA-Z]//g;s/\x01//g;s/\x02//g;s/ .*//p}')" -- $cur))
COMPREPLY=( $( compgen -W "$(bluetoothctl help | sed -n '/ /{s/\x1B\[[0-9;]*[a-zA-Z]//g;s/\x01//g;s/\x02//g;s/ .*//p}')" -- "$cur"))
  • quoting
Suggested change
COMPREPLY=( $( compgen -W "$(bluetoothctl help | sed -n '/ /{s/\x1B\[[0-9;]*[a-zA-Z]//g;s/\x01//g;s/\x02//g;s/ .*//p}')" -- $cur))
COMPREPLY=( $( compgen -W "$(bluetoothctl help | sed -n '/ /{s/\x1B\[[0-9;]*[a-zA-Z]//g;s/\x01//g;s/\x02//g;s/ .*//p;}')" -- $cur))
  • POSIX sed requires a semicolon or newline before <right-brace>.
Suggested change
COMPREPLY=( $( compgen -W "$(bluetoothctl help | sed -n '/ /{s/\x1B\[[0-9;]*[a-zA-Z]//g;s/\x01//g;s/\x02//g;s/ .*//p}')" -- $cur))
COMPREPLY=( $( compgen -W "$(bluetoothctl help | sed -n '/ /{s/'$'\e''\[[0-9;]*[a-zA-Z]//g;s/['$'\x01\x02'']//g;s/ .*//p}')" -- $cur))
  • AFAIK, POSIX sed doesn't necessarily support the escape sequences like \x1B. Here, I'd suggest expanding the sequences using Bash's $'...' before passing it to sed.

These comments also apply to the later lines that contain the same structure.

Copy link
Author

Choose a reason for hiding this comment

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

In that case, it might just be better to not use sed at all. I think this should be doable in bash only.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I think either works, so you can use what you like.

info|pair|trust|untrust|block|unblock|remove|connect|disconnect)
# Return a device to complete
COMPREPLY=( $( compgen -W "$(bluetoothctl devices|awk '{print $2}')" -- $cur))
;;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
;;
;;
  • The indentation seems to be inconsistent with the other similar lines?

Comment on lines +38 to +39
fi
if [[ "$COMP_CWORD" == "3" ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
fi
if [[ "$COMP_CWORD" == "3" ]]; then
elif ((COMP_CWORD" == 3)); then

fi
fi
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think complete -F _bluetoothctl bluetoothctl is missing.

_bluetoothctl()
{
local cur=${COMP_WORDS[COMP_CWORD]}
local prev=${COMP_WORDS[COMP_CWORD-1]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you intend to use the auto-loading mechanism of bash-completion (i.e., putting the file in the completions directory and letting bash-completion load it automatically on demand), I suggest using _init_completion for the initialization (as done in other completions). It doesn't only provide a better extraction of the words but also the contextual completions for the variable names and redirections. It might also be used by bash-completion for hooks in the future.

@audunmg
Copy link
Author

audunmg commented Apr 22, 2022

Good point, have not consulted with BlueZ, I will do that, and fix the points you mentioned first.

Is it OK if I close this, and reopen if upstream prefer it here?

Thank you for your time, I appreciate it.

@akinomyoga
Copy link
Collaborator

Is it OK if I close this, and reopen if upstream prefer it here?

Sure!

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.

2 participants