Re: [RFA] mixed source+assembly from cli disassemble

[Joel Brobecker <brobecker@adacore.com>]

Doug,

> 2008-04-03  Doug Evans  <dje@google.com>
> 
> 	* cli/cli-cmds.c (print_disassembly): New fn.
> 	(disassemble_current_function): New fn.
> 	(disassemble_command): Recognize /s modifier, print mixed
> 	source+assembly.
> 	(init_cli_cmds): Update disassemble help text.

MichaelS already approved the patch, and I agree with him. I had some
comments, and a request so I'm sending them now.

First, the request: Can you also write up a NEWS entry? I think this
is a great addition and it deserves a mention in NEWS.

The ChangeLog is missing the entry for the documentation update. Please
make sure that EliZ has seen it before checking in.  I'm not sure
whether this is the case or not anymore.

I also personally prefer to avoid abbreviations such as "fn", although
the GNU coding standards say nothing against them - so it may only be
a personally preference. I see that it has been use a few times in
the past, but most uses is in the 90s. Don't change it unless other
maintainers agree that it should be changed.

> +	  printf_filtered ("from ");
> +	  deprecated_print_address_numeric (low, 1, gdb_stdout);
> +	  printf_filtered (" to ");
> +	  deprecated_print_address_numeric (high, 1, gdb_stdout);
> +	  printf_filtered (":\n");

While you are modifying this code, I think the following should work too:

    printf_filtered ("form %s to %s:\n", paddress (low), paddress (high));

Not only does this help us get rid of a couple of uses of deprecated
functions, I believe it's also better for i18n.

Other than that, thanks for doing the work! I'll definitely enjoy
this new feature :).

-- 
Joel