On 2/20/24 10:31, Adam Julis wrote: > Signed-off-by: Adam Julis <ajulis@xxxxxxxxxx> > --- > v2: > - instead new completer is used already existing virshPoolTypeCompleter > - added flag VIRSH_POOL_TYPE_COMPLETER_COMMA for the completer > - fixed bug in pool-list --type command > > v1: > https://lists.libvirt.org/archives/list/devel@xxxxxxxxxxxxxxxxx/message/K6GT4P5S52DSRYMVQD3HLGIN33N4XPJR/ > > tools/virsh-completer-domain.h | 4 ++++ > tools/virsh-completer-pool.c | 14 +++++++++----- > tools/virsh-pool.c | 3 +++ > 3 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h > index 27cf963912..fe4af07937 100644 > --- a/tools/virsh-completer-domain.h > +++ b/tools/virsh-completer-domain.h > @@ -31,6 +31,10 @@ enum { > VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC = 1 << 0, /* Return just MACs */ > }; > > +enum { > + VIRSH_POOL_TYPE_COMPLETER_COMMA = 1 << 0, > +}; > + This doesn't need to be in virsh-completer-domain.h but can live in virsh-completer-pool.h happilly since its only purpose is to be passed to virshPoolTypeCompleter() which is declared in the latter header file. > char ** > virshDomainInterfaceCompleter(vshControl *ctl, > const vshCmd *cmd, > diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c > index 0600394411..abad0fca1b 100644 > --- a/tools/virsh-completer-pool.c > +++ b/tools/virsh-completer-pool.c > @@ -93,13 +93,17 @@ virshPoolTypeCompleter(vshControl *ctl, > g_auto(GStrv) tmp = NULL; > const char *type_str = NULL; > > - virCheckFlags(0, NULL); > - > - if (vshCommandOptStringQuiet(ctl, cmd, "type", &type_str) < 0) > - return NULL; > + virCheckFlags(VIRSH_POOL_TYPE_COMPLETER_COMMA, NULL); > > tmp = virshEnumComplete(VIR_STORAGE_POOL_LAST, > virStoragePoolTypeToString); > > - return virshCommaStringListComplete(type_str, (const char **)tmp); > + if (flags & VIRSH_POOL_TYPE_COMPLETER_COMMA){ > + if (vshCommandOptStringQuiet(ctl, cmd, "type", &type_str) < 0) > + return NULL; > + > + return virshCommaStringListComplete(type_str, (const char **)tmp); > + } > + > + return g_steal_pointer(&tmp); After playing with this for a while I found it more readable if the code is reorganized a bit: if (!(flags & VIRSH_POOL....)) return g_steal_pointer(&tmp); if (vshCommandOptStringQuiet() < 0) return NULL; return virshCommaStringListComplete(); > } > diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c > index 36f00cf643..6936406087 100644 > --- a/tools/virsh-pool.c > +++ b/tools/virsh-pool.c > @@ -1088,6 +1088,7 @@ static const vshCmdOptDef opts_pool_list[] = { > }, > {.name = "type", > .type = VSH_OT_STRING, > + .completer_flags = VIRSH_POOL_TYPE_COMPLETER_COMMA, > .completer = virshPoolTypeCompleter, Personal preference - .completer_flags can be declared after .completer. > .help = N_("only list pool of specified type(s) (if supported)") > }, > @@ -1414,6 +1415,7 @@ static const vshCmdOptDef opts_find_storage_pool_sources_as[] = { > {.name = "type", > .type = VSH_OT_DATA, > .flags = VSH_OFLAG_REQ, > + .completer = virshPoolTypeCompleter, > .help = N_("type of storage pool sources to find") > }, > {.name = "host", > @@ -1501,6 +1503,7 @@ static const vshCmdOptDef opts_find_storage_pool_sources[] = { > {.name = "type", > .type = VSH_OT_DATA, > .flags = VSH_OFLAG_REQ, > + .completer = virshPoolTypeCompleter, > .help = N_("type of storage pool sources to discover") > }, > {.name = "srcSpec", I've fixed all the small issues and merged. Congratulations on your first libvirt contribution! Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx> Michal _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
