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

Hi Kohei,

Thank you for your feedback.

On Sat, Feb 25, 2012 at 02:45, Kohei Yoshida <kohei.yoshida@suse.com> wrote:
> On Fri, 2012-02-24 at 20:38 -0500, Kohei Yoshida wrote:
> > 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.
>
> Actually, the new method doesn't have to be in ScDocument at all.  The
> only thing you need to set to ScDocument is the grammar, but other than
> that, the rest can be done outside of ScDocument.  I'm not really sure
> what the best place would be, but you could, for instance, put it in
> ScDocShell, since it needs to be callable from CheckConfigOptions...
> Just an idea.

Yes, SeDocShell seems to be a good place to put it in. I made the
necessary changes as a follow-up patch, see attached file.

I noticed that the SetFormulaOptions is also needed in
ScDocShell::InitOptions otherwise documents will not show up with the
selected formula format on open.

As a next step I will try to group the Formula options as discussed.

/Albert

From 4cc9334ecca2d64ed32e72e15a0f9272ffbbe953 Mon Sep 17 00:00:00 2001
From: Albert Thuswaldner <albert.thuswaldner@gmail.com>
Date: Mon, 27 Feb 2012 00:41:39 +0100
Subject: [PATCH] Splitting ScDocument::SetDocOptions, moved new SetFormulaOptions to ScDocShell

---
 sc/source/core/data/documen3.cxx |   32 --------------------------------
 sc/source/ui/app/scmod.cxx       |    8 ++------
 sc/source/ui/docshell/docsh3.cxx |    2 ++
 sc/source/ui/docshell/docsh6.cxx |   28 +++++++++++++++++++++++++++-
 sc/source/ui/inc/docsh.hxx       |    2 ++
 5 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/sc/source/core/data/documen3.cxx b/sc/source/core/data/documen3.cxx
index 28eab09..ddacf92 100644
--- a/sc/source/core/data/documen3.cxx
+++ b/sc/source/core/data/documen3.cxx
@@ -52,10 +52,8 @@
 #include "rangelst.hxx"
 #include "chartarr.hxx"
 #include "chartlock.hxx"
-#include "compiler.hxx"
 #include "refupdat.hxx"
 #include "docoptio.hxx"
-#include "appoptio.hxx"
 #include "viewopti.hxx"
 #include "scextopt.hxx"
 #include "brdcst.hxx"
@@ -67,7 +65,6 @@
 #include "dociter.hxx"
 #include "detdata.hxx"
 #include "detfunc.hxx"
-#include "scmod.hxx"        // SC_MOD
 #include "inputopt.hxx"     // GetExpandRefs
 #include "chartlis.hxx"
 #include "sc.hrc"           // SID_LINK
@@ -1938,39 +1935,10 @@ const ScDocOptions& ScDocument::GetDocOptions() const
 
 void ScDocument::SetDocOptions( const ScDocOptions& rOpt )
 {
-    ScAppOptions rAppOpt=SC_MOD()->GetAppOptions();
-
     OSL_ENSURE( pDocOptions, "No DocOptions! :-(" );
-    //    bool bUpdateFuncNames = pDocOptions->GetUseEnglishFuncName() != 
rOpt.GetUseEnglishFuncName();
 
     *pDocOptions = rOpt;
-
     xPoolHelper->SetFormTableOpt(rOpt);
-
-    SetGrammar( rAppOpt.GetFormulaSyntax() );
-
-    //if (bUpdateFuncNames)
-    {
-        // This needs to be called first since it may re-initialize the entire
-        // opcode map.
-        if (rAppOpt.GetUseEnglishFuncName())
-        {
-            // switch native symbols to English.
-            ScCompiler aComp(NULL, ScAddress());
-            ScCompiler::OpCodeMapPtr xMap = 
aComp.GetOpCodeMap(::com::sun::star::sheet::FormulaLanguage::ENGLISH);
-            ScCompiler::SetNativeSymbols(xMap);
-        }
-        else
-            // re-initialize native symbols with localized function names.
-            ScCompiler::ResetNativeSymbols();
-
-        // Force re-population of function names for the function wizard, function tip etc.
-        ScGlobal::ResetFunctionList();
-    }
-
-    // Update the separators.
-    ScCompiler::UpdateSeparatorsNative(
-        rAppOpt.GetFormulaSepArg(), rAppOpt.GetFormulaSepArrayCol(), 
rAppOpt.GetFormulaSepArrayRow());
 }
 
 const ScViewOptions& ScDocument::GetViewOptions() const
diff --git a/sc/source/ui/app/scmod.cxx b/sc/source/ui/app/scmod.cxx
index 9798306..f496211 100644
--- a/sc/source/ui/app/scmod.cxx
+++ b/sc/source/ui/app/scmod.cxx
@@ -1088,13 +1088,9 @@ void ScModule::ModifyOptions( const SfxItemSet& rOptSet )
     }
 
     // Do all the format updates on open documents in one go
-    if ( bUpdateDocFormat && pDoc )
+    if ( bUpdateDocFormat && pDocSh )
     {
-        const ScDocOptions& rOpt = pDoc->GetDocOptions(); // Temporary fix to keep
-                                                          // SettDocOption call as is
-
-                                                          // Needs update.
-        pDoc->SetDocOptions( rOpt );
+        pDocSh->SetFormulaOptions( *pAppCfg );
         pDocSh->SetDocumentModified();
     }
 
diff --git a/sc/source/ui/docshell/docsh3.cxx b/sc/source/ui/docshell/docsh3.cxx
index f5692ef..522e9d7 100644
--- a/sc/source/ui/docshell/docsh3.cxx
+++ b/sc/source/ui/docshell/docsh3.cxx
@@ -432,6 +432,7 @@ void ScDocShell::InitOptions(bool bForLoading)      // called from InitNew and L
     ScModule* pScMod = SC_MOD();
 
     ScDocOptions  aDocOpt  = pScMod->GetDocOptions();
+    ScAppOptions  aAppOpt  = pScMod->GetAppOptions();
     ScViewOptions aViewOpt = pScMod->GetViewOptions();
     aDocOpt.SetAutoSpell( bAutoSpell );
 
@@ -448,6 +449,7 @@ void ScDocShell::InitOptions(bool bForLoading)      // called from InitNew and L
 
     aDocument.SetDocOptions( aDocOpt );
     aDocument.SetViewOptions( aViewOpt );
+    SetFormulaOptions( aAppOpt );
 
     //  Druck-Optionen werden jetzt direkt vor dem Drucken gesetzt
 
diff --git a/sc/source/ui/docshell/docsh6.cxx b/sc/source/ui/docshell/docsh6.cxx
index c02956c..9d054bb 100644
--- a/sc/source/ui/docshell/docsh6.cxx
+++ b/sc/source/ui/docshell/docsh6.cxx
@@ -47,9 +47,9 @@
 #include "viewdata.hxx"
 #include "tabvwsh.hxx"
 #include "tablink.hxx"
-#include "appoptio.hxx"
 #include "globstr.hrc"
 #include "scmod.hxx"
+#include "compiler.hxx"
 
 #include "formula/FormulaCompiler.hxx"
 #include "comphelper/processfactory.hxx"
@@ -486,6 +486,31 @@ sal_Bool ScDocShell::ReloadTabLinks()
     return sal_True;        //! Fehler erkennen
 }
 
+void ScDocShell::SetFormulaOptions(const ScAppOptions& rAppOpt )
+{
+    aDocument.SetGrammar( rAppOpt.GetFormulaSyntax() );
+
+    // This needs to be called first since it may re-initialize the entire
+    // opcode map.
+    if (rAppOpt.GetUseEnglishFuncName())
+    {
+        // switch native symbols to English.
+        ScCompiler aComp(NULL, ScAddress());
+        ScCompiler::OpCodeMapPtr xMap = 
aComp.GetOpCodeMap(::com::sun::star::sheet::FormulaLanguage::ENGLISH);
+        ScCompiler::SetNativeSymbols(xMap);
+    }
+    else
+        // re-initialize native symbols with localized function names.
+        ScCompiler::ResetNativeSymbols();
+
+    // Force re-population of function names for the function wizard, function tip etc.
+    ScGlobal::ResetFunctionList();
+
+    // Update the separators.
+    ScCompiler::UpdateSeparatorsNative(
+        rAppOpt.GetFormulaSepArg(), rAppOpt.GetFormulaSepArrayCol(), 
rAppOpt.GetFormulaSepArrayRow());
+}
+
 void ScDocShell::CheckConfigOptions()
 {
     if (IsConfigOptionsChecked())
@@ -506,6 +531,7 @@ void ScDocShell::CheckConfigOptions()
         // separator.  Reset them to default.
         ScAppOptions aNew = rAppOpt;
         aNew.ResetFormulaSeparators();
+        SetFormulaOptions(aNew);
         pScMod->SetAppOptions(aNew);
 
         // Launch a nice warning dialog to let the users know of this change.
diff --git a/sc/source/ui/inc/docsh.hxx b/sc/source/ui/inc/docsh.hxx
index 74e12f4..488c202 100644
--- a/sc/source/ui/inc/docsh.hxx
+++ b/sc/source/ui/inc/docsh.hxx
@@ -39,6 +39,7 @@
 #include "scdllapi.h"
 #include "scdll.hxx"
 #include "document.hxx"
+#include "appoptio.hxx"
 #include "shellids.hxx"
 #include "refreshtimer.hxx"
 #include "optutil.hxx"
@@ -320,6 +321,7 @@ public:
     void            UpdateLinks();          // Link-Eintraege aktuallisieren
     sal_Bool            ReloadTabLinks();       // Links ausfuehren (Inhalt aktualisieren)
 
+    void            SetFormulaOptions(const ScAppOptions& rAppOpt );
     virtual void    CheckConfigOptions();
 
     void            PostEditView( ScEditEngineDefaulter* pEditEngine, const ScAddress& rCursorPos 
);
-- 
1.7.3.4