Re: [PATCH 3/6] attach to command_option-changed observer.

[Tom Tromey <tromey@redhat.com>]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2012-07-24  Yao Qi  <yao@codesourcery.com>
Yao> 	* NEWS: Mention new MI notification.
Yao> 	* mi/mi-interp.c: Declare mi_command_option_changed.
Yao> 	(mi_interpreter_init): Attach mi_command_option_changed to
Yao> 	observer command_option_changed.
Yao> 	(mi_command_option_changed): New.
Yao> 	* mi/mi-main.c (mi_cmd_execute): Check command 'gdb-set' and set
Yao> 	mi_suppress_notification.
Yao> 	* mi/mi-main.h: New enum 'MI_SUPPRESS_COMMAND'.

Yao> +static void
Yao> +mi_command_option_changed (const char *option, const char *value)

Needs an intro comment; something small is fine.

Yao> +  fprintf_unfiltered (mi->event_channel,
Yao> +		      "option-changed,option=\"%s\",value=\"%s\"",
Yao> +		      option, value);

This has a quoting bug.  VALUE can contain characters requiring special
quoting for MI.

I once wrote a patch to change the event channel to use ui-out:

    http://sourceware.org/ml/gdb-patches/2011-01/msg00518.html

That would be one way to deal with the problem, but there are probably
other ways as well.  (I didn't commit that patch because Volodya had a
different approach to emitting the notifications that I was interested
in... but I still think it is a decent idea.)

Yao> +  else if (strncmp (parse->command, "gdb-set", sizeof ("gdb-set") - 1 ) == 0)

There's an extra space before the closing paren.

I don't like this approach much but I understand why you did it.

Yao> +enum MI_SUPRESS_NOTIFICATION { MI_SUPPRESS_BREAKPOINT, MI_SUPPRESS_COMMAND };

Put each constant on its own line.

Tom