This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: mi tty commands


> Date: Sat, 28 May 2005 19:08:55 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Sun, May 22, 2005 at 05:00:40PM -0400, Bob Rossi wrote:
> > On Thu, Mar 10, 2005 at 09:26:44PM -0500, Bob Rossi wrote:
> > > Ping
> > 
> > Ping, and O yeah, I updated the patch the best I could to get the code
> > to conform with the GNU coding standards. I hope I'm getting this right.
> 
> Not quite, but I don't mind proofreading :-)

Sorry, I somehow managed to miss the original message.  My comments
are below.

> +  /* add the filename of the terminal connected to inferior I/O */
> +  add_setshow_string_noescape_cmd ( "inferior_tty", class_run,
> +				    &inferior_io_terminal, _("\
> +Set terminal for future runs of program being debugged."), _("\
> +Show terminal for future runs of program being debugged."), _("\
> +Usage: set inferior_tty /dev/pts/1"), NULL, NULL, &setlist, &showlist);
> +  set_cmd_completer (c, filename_completer);

You are adding a new user command, but you didn't add its
documentation to gdb.texinfo.  Please add that; I don't want us to
ever have undocumented commands again.

And I agree with Daniel: let's get rid of that underscore in the
command's name.

> +Show terminal for future runs of program being debugged.

I think the ``future'' part should be removed from this sentence: what
this command outputs is the terminal used _now_ as well as in the
future.

> +	(set_inferior_io_terminal): Add accessor definition
> +	(get_inferior_io_terminal): Add accessor definition

You don't need to write the description of the change more than once.
Here's my suggestion for how to rewrite this:

	(set_inferior_io_terminal, get_inferior_io_terminal): Add
	accessor definition.

> +	* inferior.h (set_inferior_io_terminal): Add accessor declaration
> +	(get_inferior_io_terminal): Add accessor declaration

Same here.

> +	* nto-procfs (procfs_create_inferior): Use accessor function
> +	* win32-nat.c (child_create_inferior): Use accessor function

Same here:

	* nto-procfs.c (procfs_create_inferior)
	* win32-nat.c (child_create_inferior): Use get_inferior_io_terminal.

Note how I used the explicit name of the function used, instead of
just "accessor function".


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]