Ping.  Eli, Joel requests review of the doc parts of this patch.

On Thu, Apr 17, 2008 at 2:25 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> > 2008-04-16  Doug Evans  <dje@google.com>
>  >
>  >       * NEWS: Mention new /m modifier for disassemble command.
>  >       * cli/cli-cmds.c (print_disassembly): New function.
>  >       (disassemble_current_function): New function
>  >       (disassemble_command): Recognize /m modifier, print mixed
>
> >       source+assembly.
>  >       (init_cli_cmds): Update disassemble help text.
>
>  The code part of your patch is good to go, with a couple of tiny
>  adjustments. Let's wait for Eli's feedback before checking in, since
>  there is some documentation being adjusted in the code too.
>
>
>  > +      printf_filtered ("Dump of assembler code ");
>  > +      if (name != NULL)
>  > +     {
>  > +       printf_filtered ("for function %s:\n", name);
>  > +     }
>  > +      else
>  > +     {
>  > +       printf_filtered ("from %s to %s:\n", paddress (low), paddress (high));
>  > +     }
>
>  We don't need the curly braces in this case. Can you remove them?
>
>
>  > +      if (*arg == '\0')
>  > +     error (_("Missing modifier."));
>  > +
>  > +      while (*arg && ! isspace (*arg))
>  > +     {
>  > +       switch (*arg++)
>  > +         {
>  > +         case 'm':
>
> > +           mixed_source_and_assembly = 1;
>  > +           break;
>  > +         default:
>  > +           error (_("Invalid disassembly modifier."));
>  > +         }
>  > +     }
>  > +
>  > +      while (isspace (*arg))
>  > +     ++arg;
>
>  The formatting looks weird, because some of the lines (the "while" lines
>  for instance) are using spaces instead of tabs. Could you fix that to
>  use TABs, please?
>
>  > @@ -1383,6 +1439,7 @@ With two args if one is empty it stands
>
> >    c = add_com ("disassemble", class_vars, disassemble_command, _("\
>  >  Disassemble a specified section of memory.\n\
>  >  Default is the function surrounding the pc of the selected frame.\n\
>  > +With a /m modifier source lines, if available, are included.\n\
>
>  I'd like to have Eli's feedback on this change. I would phrase
>  differently (the current form is missing a coma to make it
>  intelligible):
>
>   With a /m modifier, source lines are included (if available).


---------- Forwarded message ----------
From: Doug Evans <dje@google.com>
Date: Wed, Apr 16, 2008 at 3:25 PM
Subject: Re: [RFA] mixed source+assembly from cli disassemble
To: Joel Brobecker <brobecker@adacore.com>, Michael Snyder
<msnyder@specifix.com>, Eli Zaretskii <eliz@gnu.org>
Cc: gdb-patches@sourceware.org


Here is a revised patch with all requested changed.

 I changed the modifier to /m.  I know it was only a suggestion
 Michael, but it grew on me.

 Ok to check in?