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

Hi Albert,

On Sat, 2012-02-25 at 01:12 +0100, Albert Thuswaldner wrote:

> 1.) ScDocument::SetDocOptions requires some rethink. Here, now that
> the format option stuff is moved to appoptions, only
> xPoolHelper->SetFormTableOpt(rOpt); uses data from docoptions, the mix
> of options makes this method ugly. Splitting this up might be a
> solution, however ScDocument::SetDocOptions is used in quite many
> places....

So, your observation is correct.  As you say, it's best to take the code
that sets the formula options out of SetDocOptions and into its own
method.  We could call it SetFormulaOptions() or something.  The new
SetDocOptions() would basically have nothing other than

{
    OSL_ENSURE( pDocOptions, "No DocOptions! :-(" );
    *pDocOptions = rOpt;
    xPoolHelper->SetFormTableOpt(rOpt);
}

and the rest can go to the new method.

Also, while it's true that SetDocOptions() is called in many places, we
only need to worry about two places; one is in
ScModule::ModifyOptions(), and the other is in
ScDocShell::SetConfigOptions().  In SetConfigOptions(), we won't need to
worry about the doc options at all and only handle the formula options.
I hope this will make it easier for you.

> 2.) ScModule::ModifyOptions here improvements can possible be made to
> minimize the need for updating/repainting the open document.

Yes, although repainting can take place only in the visible region and
is done only once after the Options dialog is dismissed.  So, even if we
repaint the whole thing it's not too bad.

> 3.) ScTpFormulaOptions Class might be simplified somehow, for instance
> to "group" the options like is done for View options and Print
> Optiions(?)

Yup, I agree.  If we could somehow group the formula options like the
view options, that would improve the readability as well as it would
make it easier to extend it in future.  But you can do this as a
separate, follow-up change if you wish.

BTW, the other part of your patch looks good.  Thanks a lot for your
work.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc