Re: [PATCH] MI error messages

Jason Molenda
2005-06-22T19:19:50Z

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.