Stephan Witt <[email protected]> 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.
A few remarks below.
I am a bit lost about what happens in getConfig primarily...
JMarc
> +#ifndef THESAURUS_LOCATION
> +# define THESAURUS_LOCATION "thes"
> +#endif
Please refrain from using #defines. I do not think this particular one
is useful.
> Index: src/AspellChecker.cpp
> ===================================================================
> +# ifndef ASPELL_FRAMEWORK
> +# define ASPELL_FRAMEWORK "Aspell.framework"
> +# endif
I would be better to define this default in the configure script.
> +# ifndef ASPELL_FRAMEWORK_DATA
> +# define ASPELL_FRAMEWORK_DATA "/Resources/data"
> +# endif
> +# ifndef ASPELL_FRAMEWORK_DICT
> +# define ASPELL_FRAMEWORK_DICT "/Resources/dict"
> +# endif
> +
> +# ifndef ASPELL_MACPORTS
> +# define ASPELL_MACPORTS "/opt/local"
> +# endif
> +# ifndef ASPELL_MACPORTS_DATA
> +# define ASPELL_MACPORTS_DATA "/lib/aspell-0.60"
> +# endif
> +# ifndef ASPELL_MACPORTS_DICT
> +# define ASPELL_MACPORTS_DICT "/share/aspell"
> +# endif
Here also, I think the configure script is the right place to determine
which places may contain dictionaries. Moreover, this code may be more
platform dependent.
> struct Speller {
> - AspellSpeller * speller;
> + ///AspellSpeller * speller;
Just delete it if it is not used.
> +bool checkAspellData(AspellConfig * config,
> + char const * basepath, char const * datapath, char const * dictpath,
> + string const & lang, string const & variety)
Why use char const * here? Please stick to string for interfeces unless
really necessary.
> +AspellConfig * getConfig(string const & lang,
> + string const & variety)
Note that all the free-standing functions should be in an anonymous
namespace.
> +{
> AspellConfig * config = new_aspell_config();
> #ifdef __APPLE__
Try to find a way to avoid as much as possible this __APPLE__ special
casing. Most of the code you have here can be useful to all platforms.
> + char const * sysdir =
> lyx::support::package().system_support().absFileName().c_str() ;
> + char const * userdir =
> lyx::support::package().user_support().absFileName().c_str() ;
These variables are not used in the rest of the function... But I would
like them to be used :)
> + char const * framework = ASPELL_FRAMEWORK ;
>
> - if ( strlen(framework) && getPrivateFrameworkPathName(buf, sizeof(buf),
> framework) ) {
Please, only std::string !
