Re: patch: command deprecator

[Elena Zannoni <ezannoni at cygnus dot com>]

David Whedon writes:
 > I got the command deprecator working.  Now we can gradually turn gdb's
 > command set into the wonder we all know it can be.
 > 
 > I checked this on a lot of commands, but if something slipped through, let
 > me know.
 > 
 > David 

Thanks for your patch, David.  I haven't finished reviewing your patch
yet, i.e. I haven't checked its logic, I have only scanned it for 
formatting issues.

Unfortunately, I see that often your code doesn't adhere to the GNU
coding standards. 
Please review the required formatting at: 
http://www.gnu.ai.mit.edu/prep/standards_toc.html
and the contribution guidlines at:
http://sourceware.cygnus.com/gdb

A few more things:

When writing new functions, you should just use ISO-C, instead of K&R style.
You can also avoid the use of the PARAMS macro.
Comments too should be formatted according to the GNU standards.
I see that some diffs are just due to new lines to which spaces have been 
added, like below:

 > *************** execute_command (p, from_tty)
 > *** 1462,1468 ****
 >     if (*p)
 >       {
 >         char *arg;
 > ! 
 >         c = lookup_cmd (&p, cmdlist, "", 0, 1);
 >   
 >         /* If the target is running, we allow only a limited set of
 > --- 1463,1470 ----
 >     if (*p)
 >       {
 >         char *arg;
 > !       line=p;
 > !       
 >         c = lookup_cmd (&p, cmdlist, "", 0, 1);
 >   
 >         /* If the target is running, we allow only a limited set of
 > *************** execute_command (p, from_tty)
 > *** 1485,1495 ****
 >   	    p--;
 >   	  *(p + 1) = '\0';
 >   	}
 > ! 
 >         /* If this command has been hooked, run the hook first. */
 >         if (c->hook)
 >   	execute_user_command (c->hook, (char *) 0);
 >   
 >         if (c->class == class_user)
 >   	execute_user_command (c, arg);
 >         else if (c->type == set_cmd || c->type == show_cmd)
 > --- 1487,1500 ----
 >   	    p--;
 >   	  *(p + 1) = '\0';
 >   	}
 > ! 	
 >         /* If this command has been hooked, run the hook first. */
 >         if (c->hook)
 >   	execute_user_command (c->hook, (char *) 0);


And some diffs are not actually changing anything in the code, like
this one:


 > *** 4275,4287 ****
 >   This value is used to set the speed of the serial port when debugging\n\
 >   using remote targets.", &setlist),
 >   		     &showlist);
 > ! 
 > !   add_show_from_set (
 > !   add_set_cmd ("remotedebug", no_class, var_zinteger, (char *) &remote_debug,
 > ! 	       "Set debugging of remote protocol.\n\
 >   When enabled, each packet sent or received with the remote target\n\
 > ! is displayed.", &setlist),
 > ! 		      &showlist);
 >   
 >     add_show_from_set (
 >   		      add_set_cmd ("remotetimeout", no_class, var_integer, (char *) &remote_timeout,
 > --- 4280,4289 ----
 >   This value is used to set the speed of the serial port when debugging\n\
 >   using remote targets.", &setlist),
 >   		     &showlist);
 > !   
 > !   add_show_from_set (add_set_cmd ("remotedebug", no_class, var_zinteger, (char *) &remote_debug, "Set debugging of remote protocol.\n\
 >   When enabled, each packet sent or received with the remote target\n\
 > ! is displayed.", &setlist), &showlist);
 >   
 >     add_show_from_set (
 >   		      add_set_cmd ("remotetimeout", no_class, var_integer, (char *) &remote_timeout,



Such diffs should be avoided, they inflate the patch and make it
harder to review.

Anyway, this is just a heads up, as I said. The other maintainers and
I will review your patch further.  Stay tuned.  In the meantime you
are welcome to submit a cleaner/better formatted patch.

Thanks
Elena