Re: [RFAv2 1/6] New cli-utils.h/.c function extract_info_print_args

[Tom Tromey <tom@tromey.com>]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> cli-utils.c has a new static function extract_arg_maybe_quoted
Philippe> that extracts an argument, possibly single quoted.

I think it would be better if this handled quotes the same way that
gdb_argv (aka libiberty's buildargv) does.  That approach seems ok-ish
(maybe not perfect), and following it would mean that at least we're not
increasing the number of ways that command lines are quoted in gdb.

I guess the reason you did not simply use gdb_argv is that the trailing
regexp is the rest of the input string.

Philippe> +/* See documentation in cli-utils.h.  */
Philippe> +
Philippe> +void
Philippe> +extract_info_print_args (const char* command,
Philippe> +			 const char **args,
Philippe> +			 bool *quiet,
Philippe> +			 std::string *regexp,
Philippe> +			 std::string *t_regexp)

I tend to think this should go somewhere else, since cli-utils is more
like "low level" parsing helpers.  But I don't really know where I
guess.

Philippe> +  if (*args != NULL && **args != '\000')
Philippe> +    *regexp = extract_arg (args);

It seems to me that this should just be "*regexp = args" for backward
compatibility.  This formulation will stop at whitespace, whereas the
previous code did not.

If changing the meaning of "info types regexp" where the regexp has
whitespace is ok, then switching fully to gdb_argv would be better.  I
don't know if it is ok to do but I suspect not.

Philippe> +/* A helper function to extract an argument from *ARG.  An argument is
Philippe> +   delimited by whitespace, but it can also be optionally quoted using
Philippe> +   single quote characters.  The return value is empty if no argument
Philippe> +   was found.  */
Philippe> +
Philippe> +static std::string
Philippe> +extract_arg_maybe_quoted (const char **arg)
Philippe> +{

One question I have is whether we could just change extract_arg to do
the dequoting.  Auditing the existing calls to see if this change would
negatively impact them might not be too hard.  And, it would be nice to
make gdb's CLI a bit more regular.

Philippe> +#define INFO_PRINT_ARGS_HELP(entity_kind) \
Philippe> +"If NAMEREGEXP is provided, only prints the " entity_kind " whose name\n\
Philippe> +matches NAMEREGEXP.\n\
Philippe> +If -t TYPEREGEXP is provided, only prints the " entity_kind " whose type\n\
Philippe> +matches TYPEREGEXP.  Note that the matching is done with the type\n\
Philippe> +printed by the 'whatis' command.\n\
Philippe> +By default, the command might produce headers and/or messages indicating\n\
Philippe> +why no " entity_kind " can be printed.\n\
Philippe> +The flag -q disables the production of these headers and messages."

Constructing help text like this is i18n-unfriendly.  gdb tries to do
this correctly, though IIRC the build isn't all wired up properly and
there aren't any actual translations :-/

Maybe it can be reworded somehow.  I'm not sure if xgettext can handle
this kind of string concatenation in combination with macro expansion,
either.

Tom