[sword-devel] Sword for BPBible 1.5.12

[Troy A. Griffitts]

Ben,

Do you have more specific information about the items you mention?  More 
comments below:


Ben Morgan wrote:
> On Mon, Apr 6, 2009 at 10:00 PM, Troy A. Griffitts <scribe at crosswire.org 
> <mailto:scribe at crosswire.org>> wrote:
> 
>     Ben Morgan wrote:
> 
>         I didn't like it either. But there were two major reasons for this:
>         a) Creating a new LocaleMgr when stringmgr was was very costly
>         as BPBible keeps around a sword.conf file and then all the paths
>         in this were automatically loaded.
> 
> 
>     This should only happen on application startup and shouldn't be too
>     costly.  I believe loading locales should actually be quite fast,
>     and if not, then we have other troubles we should fix.
> 
> Well, it isn't always. I am often dealing with network drives, etc.
> And the sword.conf files I am using can have ~15-20 entries...

AugmentPath entries or what?  I still don't believe loading locales 
should take any noticeable time, and if it does, I would like to profile 
the code to make it faster.  We could load on demand the locale from the 
SWConfig, just as a first thought.


>         b) more importantly, I discovered an issue where having 1.5.11
>         locales and 1.5.12 locales around a the same time would break
>         verse lookup.
> 
> 
>     Yes, having 1.5.11 formatted locales will not work with the latest
>     code, but I don't think I'm willing to support a configuration path
>     which has both formats.  I suppose we could add a quick check for a
>     numeric value for the first book abbrev and not load the locale in
>     1.6.x if so.  Do you think it would be beneficial.  I hesitate
>     because 1.5.x will almost certainly break if it finds the new locale
>     format files.  It's almost better just to say both formats can't
>     co-exist.
> 
> Yes, but they do. You can't just say they don't. I'm talking a real case 
> that I was trying that was breaking; trying to use the modules on a 
> BibleCS installation on another computer.

The fact remains that old locale files and new locale files cannot 
operate on the same sword configuration.  Both 1.5.x and 1.6.x will fail 
if this is the case.  We can make 1.6.x ignore the 1.5.x locales, but we 
can't do anything about the 1.5.x software already on the computer.  It 
will cease to work.  The correct thing to do is to not mix locale 
formats on the same sword configuration.  I can't see how passing hard 
coded paths is a better solution.

>         BPBible provides its own locales, and it is important that these
>         are loaded, not any others. This is why I need a custom LocaleMgr.
> 
> 
> 
>     :) I'm not gonna ask why BPBible needs its own locales.
> 
> BPBible has extra things in the locales. It has stuff in them to support 
> dashes in booknames, and also the different filter options. BPBible 
> needs to be able to completely manage its own locales, without input 
> from anywhere else - that way we know that the locale will work properly.

BibleCS also has its own uilocales.d/ folder.  We augment the base sword 
locales with anything we find in uilocales.d with this call:

LocaleMgr::getSystemLocaleMgr()->loadConfigDir("./uilocales.d");

We don't actually replace the system LocaleMgr.

>     const is important in public interface methods to advertise to the
>     consumer what our contract is.  For example, if we have a method like:
> 
>     doSomething(const SWModule *) {
>     }
> 
>     we are guaranteeing to the caller that we will not modify the
>     SWModule.  Unless hasEntry is defined as const, then we will get a
>     compile error inside doSomething if we try to call mod->hasEntry()
> 
>     We can cache.  the mutable keyword is used in C++ to tell the
>     compiler that we are allowed to change this class member even in a
>     const method.
> 
> OK, I didn't realise that. I tried setting it to const, and it didn't 
> work, so I gave up with it. If you can get it working, that's good. But 
> the commit that you did for linkEntry broke for at least Matthew and me 
> - it may behave differently for different compilers.

Nothing I changed to make members const should have modified the 
behaviour of the code.  If it doesn't work, it should have nothing to do 
with whether the methods are declared const.  Maybe I missed a missed a 
signature somewhere causing the method to not override the base class. 
Not sure.  Can you give an example of the failure, or do you see the 
problem?

>     Have Chris' latest fixes also fixed your issues with these deltas?
> 
> Sorry, I haven't tried them. I would expect they would, though.
> I will probably disable r2287 as a matter of principal - I doubt any 
> frontend would work properly with multiple v11ns at the moment (I'm 
> pretty sure BPBible and Xiphos at least won't).

I believe they should come close.  Disabling them from the API won't 
make anything operate any better if your users install modules with 
alternate versification.  You'll have a much better shot at supporting 
them if you leave the registration of other versification enabled.

Thanks for the work and the report.

	-Troy.



> 
> God Bless,
> Ben
> -------------------------------------------------------------------------------------------
> Multitudes, multitudes,
>    in the valley of decision!
> For the day of the LORD is near
>    in the valley of decision.
> 
> Giôên 3:14 (ESV)
>