Re: [PATCH 1/6] new observer command_option_changed.

[Pedro Alves <palves at redhat dot com>]

On 07/25/2012 03:43 PM, Tom Tromey wrote:
>>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
> 
> Yao> The general goal of my work is that "when the internal state of GDB
> Yao> is changed, and MI frontend and/or telnet session is not aware of
> Yao> the change, notify them to keep their display up to date".
> 
> I'm fully on board with this plan.
> 
> Yao> What do you mean by "add a Python hook"?
> 
> Sorry, I wasn't clear.
> 
> Suppose we want to add a new Python event so that Python code can also
> detect parameter changes.  My supposition is that in this case we'd want
> to not filter, and just watch them all.
> 
> Yao> In terms of bandwidth, every "=option-changed" is sent once user
> Yao> types command in console, user can't enter many commands at one
> Yao> moment (one command per second and ten commands at most, I think).
> Yao> This notification should not occupy much bandwidth.
> 
> In that case, why not just report them all?
> I agree with your thinking here, that the rate is going to be quite
> low.  I'm not sure why I didn't realize that yesterday :)

The case where rate would be a bit higher is on canned sequences of
commands, like user defined functions, that could do things like
wrap a command with a set foo X; something; set foo Y;

> Reporting them all means simpler code on the gdb side and also lets us
> avoid the new set/show constructors.  I don't see a downside.

I agree.  Statically deciding which options to report changes to will
not scale.  It's about the same as deferring the reporting all
changes to a gdb of 10 years from now, when enough frontends have
requested we notify more and more options.  :-)  It also essentially
is a one way street - only you report a changes for an option, you'll
never decide to stop reporting it, for fear of that breaking some
frontend.  Best is just to either:

- report all
- report none, but instead report a single "some option changed", and
  let the frontend refresh all its state.
- let the frontend tell gdb which settings it is interested in.

The "all" option seems the simplest.

> Tom> What happens in the case where an option has a validation function that
> Tom> fails?  IIRC gdb has an internal design flaw here.
> 
> Yao> If I understand you correctly, "validation function" means cli/cli-
> Yao> setshow.c:parse_binary_operation and cli/cli-
> Yao> setshow.c:parse_auto_binary_operation.  When validation fails, an error is 
> Yao> thrown out, and observer is not called.
> 
> Sorry again for the lack of clarity.
> 
> I mean the call to c->func at the end of do_setshow_command.
> 
> There are various spots in gdb that use a hidden variable to keep the
> "real" setting in case setting the value here fails.
> 
> For example, try "set input-radix 1".  This is an error, but I think
> with your patch the MI client will see a notification saying that it was
> changed to "1".
> 
> The design flaw here is that "set" functions do their validation after
> the setting has been made, not before.

Yeah, or rather, that there is not "validate" hook, and the "set" hook
is abused for that.

> One approach to this would be to fix this design flaw.  That's likely to
> be a lot of work...

I don't think there any that many cases to fix.

> Another approach might be to move the observer notification later.

-- 
Pedro Alves