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

Hi Albert,

On Mon, 2012-02-27 at 20:31 +0100, Albert Thuswaldner wrote:
> Hi Kohei,
>
> On Mon, Feb 27, 2012 at 18:34, Kohei Yoshida <kohei.yoshida@suse.com> wrote:
> > On Mon, 2012-02-27 at 12:15 -0500, Kohei Yoshida wrote:
> > > Hi Albert,
> > >
> > > Pushed both of your patches to master without modifications.
> >
> > Actually, now I have an issue with changing the formula syntax in the
> > UI.  The new value won't stick for some reason.
> >
> > I'm debugging this at the moment.
> Oh, sorry for that. It is probably due to one of the conversions being
> incorrect. Around line 1046 in scmod.cxx you have this:
>
>  if (rOptSet.HasItem(SID_SC_OPT_FORMULA_GRAMMAR, &pItem))
>     {
>         sal_uInt16 nVal = static_cast<const SfxUInt16Item*>(pItem)->GetValue();
>         ::formula::FormulaGrammar::Grammar eOld = pAppCfg->GetFormulaSyntax();
>         ::formula::FormulaGrammar::Grammar eNew;
>
>         switch (nVal)
>         {
>         case 0:
>             eNew = ::formula::FormulaGrammar::GRAM_NATIVE;
>             break;
>         case 1:
>             eNew = ::formula::FormulaGrammar::GRAM_NATIVE_XL_A1;
>             break;
>         case 2:
>             eNew = ::formula::FormulaGrammar::GRAM_NATIVE_XL_R1C1;
>             break;
>         }
>
> Here nVal should somehow be converted to from sal_uInt16 to
> ::formula::FormulaGrammar::Grammar (eNew). I fiddled around quite abit
> with this and think I never got it right. The above is of course
> completely wrong, it was a short-cut I was thinking about at the time,
> I storing the position of the drop down list as an integer, which I
> know is NOT the way to do it.
>
> Some kind of static_cast should be used I guess but how?

No worries.  I got to the bottom of it, and fixed it.  While I was at
it, I also found & fixed several others as well (that I didn't detect
during my initial review :-P).

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7eb1c1e31dac3b8377307192cae9cdde0ebe18bb

The punch line is that, the logic of resetting and committing
application options is handled well by the parent dialog code, the tab
page code doesn't have to store a copy of ScAppOptions at all.  This is
unlike how we handle ScDocOptions.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc