Add bash completions #62
Conversation
|
I'd probably add a hidden function to sdbootutil that can be eval'd from the completion script to avoid duplicating all that code |
I've looked a bit into how bash completion works and it seems like the bash completion has to happen in the same shell context. So I can A) source the sdbootutil script, but that would execute the functions that might fail as the bash completion can be called as non-super user Or do you have a different idea? |
TobiPeterG
force-pushed
the
bash-completion
branch
from
43e9277
to
cbd35fe
Compare
|
something like |
TobiPeterG
force-pushed
the
bash-completion
branch
3 times, most recently
from
a5d7b01
to
c4d47d4
Compare
I've tried to implement this. I couldn't test it yet though, but please tell me if this is what you had in mind :) |
TobiPeterG
force-pushed
the
bash-completion
branch
4 times, most recently
from
a93f9a9
to
9e50d0c
Compare
TobiPeterG
force-pushed
the
bash-completion
branch
3 times, most recently
from
da4439b
to
7e20590
Compare
TobiPeterG
force-pushed
the
bash-completion
branch
3 times, most recently
from
92f03a7
to
b64cdb5
Compare
TobiPeterG
force-pushed
the
bash-completion
branch
4 times, most recently
from
40a7578
to
588ed4b
Compare
|
@aplanas can you have a look at this one please? :) |
completion/sdbootutil
Outdated
| declare -ga result | ||
| result=() | ||
|
|
||
| for fn in ""/usr/lib/modules/*/"$image"; do |
There was a problem hiding this comment.
Is this correct? I would expect "/usr/lib/modules/*/$image";
There was a problem hiding this comment.
I know that $image (among other symbols) but it is confusing. We have global vars in sdbootutil, and other global vars are also exported.
We should normalize then somehow, as it is not clear when a refactor in sdbootutil will break the completion
There was a problem hiding this comment.
I removed the first two quote signs, but this seems to be correct. At least shellcheck doesn't like it otherwise.
There was a problem hiding this comment.
Interesting. But ""/... I parsed as "append the empty string to /..". It is not 'wrong', so shellcheck cannot complain, but I guess that it is not what you want to do?
There was a problem hiding this comment.
No, it's not. But as it seems, we can't just use single quotes around everything as this would run only once (at least shellcheck says that)
There was a problem hiding this comment.
Ah, so there is a complain for "/usr/lib/modules/*/$image"; from shellcheck! What is the ID?
There was a problem hiding this comment.
| local i=0 | ||
|
|
||
| for word in "${words[@]:1:$cword-1}"; do | ||
| if [ -n "${commands["$word"]+yes}" ]; then |
There was a problem hiding this comment.
I think yes.
It should match all commands with a key, not only those with a value
TobiPeterG
force-pushed
the
bash-completion
branch
4 times, most recently
from
5b8c2a1
to
2857228
Compare
This will complete started commands and provide a list of possible commands and options.
TobiPeterG
force-pushed
the
bash-completion
branch
from
2857228
to
4ab8cdc
Compare
|
@TobiPeterG the PR is looking good. Give me some time to test it a bit |
|
Should I just rebase or do you have any issues with the functionality/code that I should fix with a rebase? |
|
Yes please, do a rebase. But I need to research a bit more. I found a good reference |
| [force-update]="" | ||
| [add-kernel]=kernel | ||
| [remove-kernel]=kernel | ||
| [set-default-snapshot]="" |
There was a problem hiding this comment.
I think this has a parameter
| [list-snapshots]="" | ||
| [list-entries]="" | ||
| [list-kernels]="" | ||
| [show-entry]="" |
There was a problem hiding this comment.
I think this also has parameters
This will complete started commands and provide a list of possible commands and options.
Closes #61
Unfortunately, we can't access the available snapshots, so the user still has to know which ones are available on his system. Equally, we can't enter snapshots to find kernel versions, so we recommend only the currently installed kernels.