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.
>
> > + 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.
>
> Best,
> Quentin
>
> > + esac
> > + ;;
> > + *)
> > + [[ $prev == $object ]] && \
> > + COMPREPLY=( $( compgen -W 'dump help' -- "$cur" ) )
> > + ;;
> > + esac
> > + ;;
> > cgroup)
> > case $command in
> > show|list)
> >
>
