Re: [Libreoffice] [PATCH] Moving misplaced Compatibility options from ScDocOptions to ScAppOptions

Hi Albert,

Good to see you being active again. :-)

On Fri, 2012-02-03 at 11:47 +0100, Albert Thuswaldner wrote:

> I have updated the patch according to Markus comments above, removing
> the check to see if the keybinding option has been modified. A side
> effect of the above is that return type sal_Bool of
> ScTpCompatOptions::FillItemSet is not needed anymore (at the moment i
> have set it to always return true). In the next iteration I plan to
> check what to do with this.

I admit I'm not 100% sure, but I'm pretty sure that FillItemSet is still
used by the parent dialog code that implements the whole Options dialog.
In each option tab page, Reset() and FillItemSet() appear to provide the
modify / reset mechanism that the Options dialog need.

> Before I do that it would be nice If someone could have a look at my
> new patch. It is good, I plan to do the same thing to the Formula and
> Defaults options.

It's good for the most part.  The only thing is that, for app options,
we appear to handle it very differently than doc options or view
options.  So we'll need to modify that part to fit our current
mechanism.  I'm currently looking into this as I'm not yet really 100%
knowledgeable on how we handle this, and I also need to learn this in
order to review your patch.  Let me spend some time on this, and will
get back to you.

> BTW: will the work on simplified C++ configuration access have any
> impact on this part of the code?

Nope.  I don't think this code is affected.

Anyway, please give me some time to study the code, and I'll get back to
you.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc