On Tue, Oct 17, 2023 at 06:23:47PM +0200, David Marchand wrote:
> On Tue, Oct 17, 2023 at 10:29 AM Bruce Richardson
> <bruce.richard...@intel.com> wrote:
> > >
> > > - testpmd accepts both "help" and "help <section>" commands.
> > > But the cmdline library does not provide a way (afair) for specifiying
> > > this "optional" aspect.
> > >
> > > And it can't be expressed with this series current syntax since
> > > generated symbols would conflict if we ask for two "help" commands.
> > >
> > > My quick hack was to introduce a way to select the prefix of the
> > > generated symbols.
> > > There may be a better way, like finding a better discriminant for
> > > naming symbols...
> > > But, on the other hand, the symbols prefix might be something a
> > > developer wants to control (instead of currently hardcoded cmd_
> > > prefix).
> > >
> > > @@ -20,6 +20,12 @@ def process_command(tokens, cfile, comment):
> > > """Generate the structures and definitions for a single command."""
> > > name = []
> > >
> > > + prefix, sep, cmd_name = tokens[0].partition(':')
> > > + if cmd_name:
> > > + tokens[0] = cmd_name
> > > + else:
> > > + prefix = 'cmd'
> > > +
> > > if tokens[0].startswith('<'):
> > > print('Error: each command must start with at least one
> > > literal string', file=sys.stderr)
> > > sys.exit(1)
> > >
> > > (etc... I am not copying the rest of the diff)
> > >
> > > I then used as:
> > >
> > > cmd_brief:help # help: Show help
> > > help <STRING>section # help: Show help
> > >
> >
> > Interesting. I actually had no plans to even consider moving something like
> > testpmd over. However, this is an interesting one, though I'm not really
>
> Given the extensive use of the cmdline library in testpmd, it is a
> good way to identify the limits of this series :-).
>
>
> > sure I like it that much as a feature :-) I rather like having unique
> > prefixes for each command. I wasn't actually aware of the testpmd "help
> > <section>" command at all. I will have to look into it.
>
> Let me propose an alternative hack.
> I mentionned previously that we could have a better namespace /
> discriminant for those symbols, and it seems easier than I thought:
>
> @@ -25,8 +25,10 @@ def process_command(tokens, cfile, comment):
> sys.exit(1)
> for t in tokens:
> if t.startswith('<'):
> - break
> - name.append(t)
> + t_type, t_name = t[1:].split('>')
> + name.append(t_name)
> + else:
> + name.append(t)
> name = '_'.join(name)
>
> result_struct = []
>
> With this, any command implementation symbol has the full chain of
> token names as a prefix which will ensure there is no conflict.
> WDYT?
>
Having thought a little more about it, I still don't like having the full
command in all cases, but I can see it being useful for cases of
overlapping prefixes.
How about making it optional - setting a flag in the typename, or in the
parameter name to indicate that it should be included in the overall
command name. For example, if we prefix the variable name with "_" or "__",
it could indicate that we can choose to include this.
show port <UINT16>n --> void cmd_show_port_parsed(...)
show port <UINT16>_n --> void cmd_show_port_n_parsed(...)
Prefixes on strings beyond initial tokens could just be silently stripped.
[Obviously open to other ideas on what form tagging could take]
/Bruce
