-
Notifications
You must be signed in to change notification settings - Fork 380
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
Brctl #866
base: main
Are you sure you want to change the base?
Brctl #866
Conversation
daed251
to
7e42273
Compare
Why? With |
completions/brctl
Outdated
@@ -1,4 +1,19 @@ | |||
# bash completion for brctl -*- shell-script -*- | |||
_brctl_interfaces() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have some new naming conventions for functions in effect, please refer to https://github.com/scop/bash-completion/blob/master/doc/api-and-naming.md#naming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this one's going to be _comp_xfunc_brctl_interfaces
? I mean, there is no other consumer now, but makes perfect sense to have a function that returns a list of bridged interfaces. Then, I believe, a check like [[ -x brctl ]] || return
needs to be added in case brctl isn't installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but then again we can also do that when there is a known consumer for that function. No strong opinions.
The current implementation of the function already redirects stderr to /dev/null
, so I don't think a separate existence/executability check is required. We tend to not sprinkle those unless there's a specific reason to.
Also, the brctl to invoke should be passed to the function as a parameter. Random in-tree example:
bash-completion/completions/python
Line 6 in 8490bf8
"$("${1:-python}" "${BASH_SOURCE[0]%/*}/../helpers/python" "$cur" \ bash-completion/completions/python
Line 39 in 8490bf8
_comp_xfunc_python_modules "$1"
completions/brctl
Outdated
if [[ -z "$1" ]]; then | ||
brctl show | ||
else | ||
brctl show "$1" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if [[ -z "$1" ]]; then | |
brctl show | |
else | |
brctl show "$1" | |
fi | |
brctl show ${1:+"$1"} |
When adding a network interface to a bridge we most probably want to use one that hasn't been configured yet.
When completing a delif command offer only interfaces that are part of the selected bridge.
The show subcommand accepts a bridge name as a paramenter so let's offer completion. On the other hand addbr creates new interface and requires a name that doesn't exist yet. Longer awk condition prevents offering interfaces that are parts of bridges. They are all displayed in the last column but with all other columns being blang on the second and following lines of each bridge the last column is also the first.
Alternatively
addbr
could also get completion of available bridges to enable avoiding clashes.