Re: Code Review: Next step to provide spellchecker with dictionaries included

[Stephan Witt]

Am 31.05.2010 um 14:37 schrieb Stephan Witt:

> Am 31.05.2010 um 13:00 schrieb Jean-Marc LASGOUTTES:
> 
> Thanks for commenting!
> 
>> Stephan Witt <st.w...@gmx.net> writes:
>>> I've prepared a patch to change hunspell and myThes wrappers to be ready 
>>> for included dictionaries.
>>> Please have a look at the patch and point me to typos and thinkos.
>>> With the changes the binary searches the data files in this order:
>>> 1. user path from configuration
>>> 2. user support directory and
>>> 3. system support directory - there will the shipped data files live.
>> 
>>> +#ifndef THESAURUS_LOCATION
>>> +# define THESAURUS_LOCATION "thes"
>>> +#endif
>> 
>> Please refrain from using #defines. I do not think this particular one
>> is useful.
> 
> I know that defines are bad. But here I don't know how to make the used 
> location clear.
> The alternative is a local const string in method
> pair<string,string> Thesaurus::Private::getThesaurus(docstring const & lang)
> around line 120 of src/Thesaurus.cpp
> That's pretty far away of the beginning of the file.
> 
> Any idea?

Is something like the attached patch better?

Stephan


Index: src/Thesaurus.cpp
===================================================================
--- src/Thesaurus.cpp   (Revision 34556)
+++ src/Thesaurus.cpp   (Arbeitskopie)
@@ -42,10 +42,6 @@
 
 } // namespace anon
 
-#ifndef THESAURUS_LOCATION
-# define THESAURUS_LOCATION "thes"
-#endif
-
 struct Thesaurus::Private
 {
        ~Private()
@@ -77,6 +73,8 @@
 
        /// the thesauri
        Thesauri thes_;
+       const string dataDirectory(void) { return "thes"; }
+
 };
 
 
@@ -125,11 +123,11 @@
                result = getThesaurus(thes_path, lang);
        }
        if (result.first.empty() || result.second.empty()) {
-               string const sys_path = 
external_path(addName(lyx::support::package().system_support().absFileName(),THESAURUS_LOCATION))
 ;
+               string const sys_path = 
external_path(addName(lyx::support::package().system_support().absFileName(),dataDirectory()))
 ;
                result = getThesaurus(sys_path, lang);
        }
        if (result.first.empty() || result.second.empty()) {
-               string const user_path = 
external_path(addName(lyx::support::package().user_support().absFileName(),THESAURUS_LOCATION))
 ;
+               string const user_path = 
external_path(addName(lyx::support::package().user_support().absFileName(),dataDirectory()))
 ;
                result = getThesaurus(user_path, lang);
        }
        return result;