Re: mi tty commands

[Daniel Jacobowitz <drow at false dot 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 :-)

> +2005-05-22  Bob Rossi  <bob@brasko.net>
> +
> +	* fork-child.c (fork-inferior): Use accessor function for
> +	inferior_io_terminal
> +	* infcmd.c (inferior_io_terminal): make static

All ChangeLog entries should start with a capital letter, end with a
period, and be generally sentencelike.  You've got the third bit down.

> +	(set_inferior_io_terminal): Add accessor definition
> +	(get_inferior_io_terminal): Add accessor definition
> +	(tty_command): Use accessor function
> +	(_initialize_infcmd): Add inferior_tty setshow variable
> +	* inferior.h (set_inferior_io_terminal): Add accessor declaration
> +	(get_inferior_io_terminal): Add accessor declaration
> +	* nto-procfs (procfs_create_inferior): Use accessor function
> +	* win32-nat.c (child_create_inferior): Use accessor function
> +	* mi/mi-cmd-env.c (mi_cmd_inferior_tty_set): Add MI definition
> +	(mi_cmd_inferior_tty_set): Add MI definition

Just "New function" is usually fine.

> +	* mi/mi-cmds.c (mi_cmds): Add inferior-tty-set/inferior-tty-show

Please use "and" instead of /.

> +  const char *inferior_io_terminal = get_inferior_io_terminal();

That pesky space! :-)

>    /* If no exec file handed to us, get it from the exec-file command
>       -- with a good, common error message if none is specified.  */
> @@ -261,7 +262,7 @@
>  
>    /* Tell the terminal handling subsystem what tty we plan to run on;
>       it will just record the information for later.  */
> -  new_tty_prefork (inferior_io_terminal);
> +  new_tty_prefork ((char*)inferior_io_terminal);

Another pesky space; please use "char *".

> +  if (inferior_io_terminal)
> +    xfree ( inferior_io_terminal );

Some pesky anti-spaces.

> +  /* 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);
> +

Ditto on the pesky spaces.  The Usage: bit is unnecessary.  There are
no other set commands with underscores; inferior-tty would be better,
but would just "tty" be OK?  That's even simpler.

> -/* File name for default use for standard in/out in the inferior.  */
> +/* Set/Get name for default use for standard in/out in the inferior.  */

Get doesn't need to be capitalized here.  Also, it's still a file name;
could you leave that?

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

I think there's either one or two missing uses of "the" here; at least
one before program.

> +/* Set the inferior terminal device name */

Full sentence with a full stop at the end, please.

> +    # test that the commands,
> +    #   -inferior-tty-set
> +    #   -inferior-tth-show

Test that they what?  Or just "Test the commands:".

I tried to point out one example of each of the formatting problems;
that wasn't an exhaustive list, but you can probably see all the others
now.

-- 
Daniel Jacobowitz
CodeSourcery, LLC