rgheck <[EMAIL PROTECTED]> writes:
> What I said was misleading (=wrong). The change was actually to
> getStatus(). In this case, we want to return right away, because the
> return value says whether we handled it or not. There shouldn't be
> anything to do once we've made that decision. OK?
In the cases where a temporary 'enable' variable is chosen, there are
things to do afterwards. In general the style of doing 'break' and
"return true" at the end work very well. I do not like long functions
with a lot of exit points. It also makes debugging (how to stop at the
end of the method?) painful.
> I did something like what I thought you were suggesting. If I
> interpreted you wrongly, please clarify.
It looks good to me. Later we will need to use the scheme uniformly
(get rid of the hardcoded error handling in LyXFunc.cpp)
> I tried removing LFUN_BUFFER_PRINT, and it made no difference at all.
> What's going on seems to be this. Commands that need a buffer get
> disabled earlier in the sequence if there's no buffer. But very early
> on we have:
> bool enable = true;
> So things only get disabled if there is a reason. In the default
> branch, we also have:
> // If we do not have a BufferView, then other functions are disabled
> if (!view()) {
> enable = false;
> break;
> }
> So again things get disabled if there's no view.
I think that the default branch handling should do 'enable=false' if
nobody claimed ownership of the lfun.
> That said, things that are handled in Buffer::dispatch() should
> probably appear in Buffer::getStatus(), yes?
Yes.
The patch looks good to me (although I did not try it), except for
BufferView::getStatus (see above).
JMarc
