Re: [PATCH] gdb: improve usage strings

[Mike Frysinger <vapier at gentoo dot org>]

On Saturday 11 August 2012 13:16:18 Eli Zaretskii wrote:
> > From: Mike Frysinger <vapier@gentoo.org>
> > Date: Sat, 11 Aug 2012 12:54:40 -0400
> >    c = add_com ("signal", class_run, signal_command, _("\
> > 
> > -Continue program giving it signal specified by the argument.\n\
> > -An argument of \"0\" means continue program without giving it a
> > signal."));
> > +Continue program by sending it the specified signal.\n\
> 
> This "by sending it" is AFAIU inaccurate: we don't continue program
> _by_ sending it the signal, we continue the program _and_ send it the
> signal.  I actually don't see anything wrong with the original
> wording.

ok, but your response shows what i was trying to fix:
	- adding "the" before "program"
	- changing "giving" to "sending"
the "by" change was incidental

so i'll change it to:
	Continue program and send it the specified signal.

> >    add_com ("finish", class_run, finish_command, _("\
> >  Execute until selected stack frame returns.\n\
> > +Usage: finish\n\
> >  Upon return, the value returned is printed and put in the value
> >  history."));
> 
> Does this "usage" really add any information?

not really, but i added it for two reasons:
 - consistency: the current *lack* of help text consistency is extremely 
confusing especially
 - many commands take arguments even though the help text implies otherwise 
leaving the user to wonder "does this actually take an option?".  so even a 
usage string showing it takes no arguments is useful.

although it's funny you highlight this command because, when i was updating 
this text, i checked the command to see if it actually secretly took 
arguments.  this is what i found:

static void
finish_command (char *arg, int from_tty) 
{
  ...

  /* Find out whether we must run in the background.  */
  if (arg != NULL)
    async_exec = strip_bg_char (&arg);

  /* If we must run in the background, but the target can't do it,
     error out.  */
  if (async_exec && !target_can_async_p ())
    error (_("Asynchronous execution not supported on this target."));

  /* If we are not asked to run in the bg, then prepare to run in the
     foreground, synchronously.  */
  if (!async_exec && target_can_async_p ())
    {
      /* Simulate synchronous execution.  */
      async_disable_stdin ();
    }

  if (arg)
    error (_("The \"finish\" command does not take any arguments."));
 ...

so it seems like finish *does* secretly accept options, but in this case it's 
trying to be secret about it rather than someone just didn't fully document 
it.  or i read the "arg" parsing logic above incorrectly.

> >    add_com ("next", class_run, next_command, _("\
> >  Step program, proceeding through subroutine calls.\n\
> > +Usage: next [N]\n\
> >  Like the \"step\" command as long as subroutine calls do not happen;\n\
> >  when they do, the call is treated as one instruction.\n\
> > -Argument N means do this N times (or till program stops for another \
> > +Argument N means step N times (or till program stops for another \
> 
> Isn't it better to say "N source lines"?

hmm, probably.  it can be a little confusing when the source debug information 
isn't available for functions, so the "next" ends up skipping too much.  but 
maybe that topic is too large to cover here, so wrapping it up into "source 
lines" should work.

> Btw, I find this entire doc string completely obfuscated.  How about
> this instead:
> 
>   Step program until it reaches a different source line.
>   Usage: next [N]
>   Unlike "step", if the current source line calls a subroutine,
>   this command does not enter the subroutine, but instead steps over
>   the call, in effect treating it as a single source line.

SGTM
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.