> comments? ok to put in?
>
> ed.
>
>+ int const x = d->mouse_position_cache_.x_;
>+ int const y = d->mouse_position_cache_.y_;
>+ Inset const * covering_inset = getCoveringInset(buffer_.text(), x, y);
>+ if (!covering_inset)
>+ return;
>+
No, .. the rest of the function has to be executed even if covering_inset is 0.
>+ if ((covering_inset->asInsetCommand()
>+ && covering_inset->lyxCode() != TOC_CODE
>+ && covering_inset->lyxCode() != FLOAT_LIST_CODE
>+ && covering_inset->lyxCode() != INDEX_PRINT_CODE)
>+ || covering_inset->lyxCode() == GRAPHICS_CODE
>+ || covering_inset->lyxCode() == VSPACE_CODE
>+ || covering_inset->lyxCode() == LINE_CODE) {
>+ hoveringinset_ = true;
>+ } else if (covering_inset->asInsetCollapsable()) {
>+ FuncRequest fr = FuncRequest(LFUN_NOACTION, x, y,
>mouse_button::none);
>+ hoveringinset_ = static_cast<InsetCollapsable const
>*>(covering_inset)->hitButton(fr);
>+ }
Hmm.. this is pretty awful code :(.. I would expect something like:
hoveringinset_ = covering_inset->clickable(x,y);
This code doesn't work for InsetExternal (PS hovering doesn't work for
InsetExternal too), InsetSpace, InsetPrintIndex (clickable if there
are multiple indices). In short, I think for InsetCollapsable we
should call hitButton(), and for InsetCommands we should call
hasSettings().
>+ ///
>+ bool hoveringInset() { return hoveringinset_; }
const.
>+ ///
>+ bool hoveringinset_;
I think this is not the best naming as the variable explicitly says
something about whether we hover over a clickable inset or the
clickable part of an inset. Moreover, I would put this boolean in
BufferView::Private next to BufferView::Private::last_inset (which is
quite related, that's why. Although it's ugly to need a public
function that gives access to it :S).
Vincent
