2019-04-25 09:14 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>
> On Thu, Apr 25, 2019 at 4:15 AM Quentin Monnet
> <quentin.monnet@xxxxxxxxxxxxx> wrote:
>>
>> 2019-04-24 22:03 UTC-0700 ~ <andrii.nakryiko@xxxxxxxxx>
>>> From: Andrii Nakryiko <andriin@xxxxxx>
>>>
>>> Add full support for btf command in bash-completion script.
>>>
>>> Cc: Quentin Monnet <quentin.monnet@xxxxxxxxxxxxx>
>>> Cc: Yonghong Song <yhs@xxxxxx>
>>> Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
>>> Cc: Alexei Starovoitov <ast@xxxxxx>
>>> Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
>>> ---
>>> tools/bpf/bpftool/bash-completion/bpftool | 46 +++++++++++++++++++++++
>>> 1 file changed, 46 insertions(+)
>>>
>>> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
>>> index 9f3ffe1e26ab..030f81bdec6a 100644
>>> --- a/tools/bpf/bpftool/bash-completion/bpftool
>>> +++ b/tools/bpf/bpftool/bash-completion/bpftool
>>> @@ -217,6 +217,7 @@ _bpftool()
>>> done
>>> cur=${words[cword]}
>>> prev=${words[cword - 1]}
>>> + pprev=${words[cword - 2]}
>>>
>>> local object=${words[1]} command=${words[2]}
>>>
>>> @@ -607,6 +608,51 @@ _bpftool()
>>> ;;
>>> esac
>>> ;;
>>> + btf)
>>> + local PROG_TYPE='id pinned tag'
>>> + local MAP_TYPE='id pinned'
>>> + case $command in
>>> + dump)
>>> + case $prev in
>>> + $command)
>>> + COMPREPLY+=( $( compgen -W "id map prog file" -- \
>>> + "$cur" ) )
>>> + return 0
>>> + ;;
>>> + prog)
>>> + COMPREPLY=( $( compgen -W "$PROG_TYPE" -- "$cur" ) )
>>> + return 0
>>> + ;;
>>> + map)
>>> + COMPREPLY=( $( compgen -W "$MAP_TYPE" -- "$cur" ) )
>>> + return 0
>>> + ;;
>>> + id)
>>> + case $pprev in
>>> + prog)
>>> + _bpftool_get_prog_ids
>>> + ;;
>>> + map)
>>> + _bpftool_get_map_ids
>>> + ;;
>>> + esac
>>> + return 0
>>> + ;;
>>> + *)
>>> + if _bpftool_search_list 'map'; then
>>> + COMPREPLY+=( $( compgen -W 'key value kv all' -- \
>>> + "$cur" ) )
>>
>> Hi Andrii,
>>
>> This COMPREPLY will suggest completion for "key|value|kv|all", even if
>> one of those words has been used on the command line before already (I
>> do not believe this is expected?). What about the following instead?
>>
>> _bpftool_one_of_list 'key value kv all'
>
> I'll make it more precise and correct. key|value|kv|all should go only
> after `bpftool btf dump map id <id> <here>`, I'll do match based on
> number of workds, similar to how it's done in prog attach/detach
> handling.
I think that bit was mostly correct. What you have is “if we're after
map and it's not prog/map/id, always complete with key/value/kv/all”.
What I'm proposing is “[in the same context], complete just once with
only one of key/value/kv/all”. Like this:
*)
if _bpftool_search_list 'map'; then
_bpftool_one_of_list 'key value kv all'
fi
With that you probably do not have to change it for word numbers.
Although if you wish to go for them, it's fine too.
>
>
>>
>>> + fi
>>> + return 0
>>> + ;;
>>
>> Nit: It seems that the last bloc (the case when $prev matches on "*") is
>> not correctly indented, it should be aligned with $command/prog/map/id?
>
> Copy/pasted from `prog dump`, I wasn't sure if that was intentional,
> will fix both.
>
>>
>> Other than this the completion seems good to me.
>>
>> But note that I did not receive all of your patches, only the cover
>> letters (for v1 and v2 of the set) and this one (because I'm directly
>> CC-ed?). It looks like the other patches for both v1 and v2 did not make
>> it to the mailing lists or to patchwork, so you might want to double
>> check and resend if necessary.
>
> Hm.. yeah, you are right, not sure what happened there. Neither bfp@
> and netdev@ didn't get it, while other recipients did. I'll send out
> v3 and will see if that still happens.
Cool, thanks!
Quentin
