Re: [PATCH] MI error messages

[Jason Molenda <jmolenda at apple dot com>]

Hi Nick,

This patch is changing two things.  You're not using error () when  
there is an error, and you're changing the error message texsts.

Why are you avoiding error()?  It's use throughout gdb is  
established; there are people who like this style of error handling  
and people who don't, but consistency is valuable and we're using  
error().  I'm not clear on why you're patching it out of mi-cmd- 
var.c.  Is your goal to have the error message output as just ^error  
w/o the console-quoted error text?  Using your mi_error will result  
in some error messages (i.e. from the rest of gdb) going through error 
() and some other error messages (from the mi/ sub directory) using  
mi_error(), so the front end will have both.

Second, the error messages are being changed from e.g.

"mi_cmd_var_create: Usage: NAME FRAME EXPRESSION."

to

"-var-create NAME FRAME EXPRESSION."

Why is this better?  Neither one means anything to an end user --  
they're just strings for programmers to grep for in the gdb sources,  
and to disambiguate the same error message from different mi  
commands.  Some of the error message changes are good improvements  
(e.g. adding the symbol that was being var-create'd in  
"mi_cmd_var_create: unable to create variable object." but you  
dropped the mi command name, either function or MI command, entirely  
in that case), but there are a lot of changes in there.

(I didn't mention the third change, which is to flag mi command usage  
error separately, but you can indicate that in the error text already  
so I didn't see the compelling reason for that change either...)

We changed error () here at Apple so it is only printed as an MI  
^error msg; our error_stream() reads


  /* Copy the stream into the GDB_LASTERR buffer.  */
  ui_file_rewind (gdb_lasterr);
  ui_file_put (stream, do_write, gdb_lasterr);

  /* Write the message plus any error_pre_print to gdb_stderr.
     Don't do it for mi like interpreters, however, since
     the will get the error from ui_file_rewind. */
  if (!ui_out_is_mi_like_p (uiout))
    {
      target_terminal_ours ();
      wrap_here ("");           /* Force out any buffered output */
      gdb_flush (gdb_stdout);
      annotate_error_begin ();
      if (error_pre_print)
        fputs_filtered (error_pre_print, gdb_stderr);
      ui_file_put (stream, do_write, gdb_stderr);
      fprintf_filtered (gdb_stderr, "\n");
    }

Jason

PS-  If you add "-p" to your $HOME/.cvsrc, e.g.
diff -up
it will make the diffs a tiny bit easier to read.  -p makes diff try  
to figure out the function name and include it in the diff so people  
can easily tell which function the change is against.