Getting a good night’s sleep is important

Today, I finally found the time to read up on some rather ancient e-mails. One was from Paolo in the dxgettext mailing list about a bug in gnugettext.pas:

With SVN revision 75 and the following, AddDomainForResourceString() has no effect, additional translations are not used.

Suggested patch:

original code from SVN revision 79:

function TGnuGettextInstance.LoadResString(
  ResStringRec: PResStringRec): UnicodeString;
begin
  Result:=Gettext(GetResString(ResStringRec));
end;

patched code:

function TGnuGettextInstance.LoadResString(
  ResStringRec: PResStringRec): UnicodeString;
begin
  Result:=ResourceStringGettext(GetResString(ResStringRec));
end;

He is right: After the change I made in revision 75 AddDomainForResourceString no longer worked as expected. I had never experienced that problem because I don’t use resource strings but prefer calling _() explicitly.

Having had not much sleep last night (at least that’s a good excuse because it’s even true), I simply took the suggested patch and committed it. This turned out to be a bad idea because it was reverting the change I made in revision 75 back in July 2016. That change fixed a bug that “Der schöne Günther” mentioned in Delphi Praxis. Unfortunately that bugfix introduced the new bug.

Good thing that Obones apparently had slept well 😉 and spotted the problem immediately:

Before changing this back, please consider where it comes from:
https://sourceforge.net/p/dxgettext/code/75
http://www.delphipraxis.net/1333593-post1.html

I’m not sure which one is correct, but going back to the global instance in an method of the instance itself seems a bit counterintuitive.
If one wants to call .LoadResString, I believe one should make sure it works on the expected domain.
But if one wants to have the support for multiple domains, then no specific instance method should be called.

My 2 cents.

That woke me up and I actually went and wrote a test program based on the code which Der schöne Günther posted. Then I fixed the bug, or actually, both:

As of revision 83 TGnuGettextInstance has a method ResourceStringGettext which takes both, the selected language of that instance as well as the domains added with AddDomainForResourceString into account. The fix was even quite simple: Just move the code from the global ResourceStringGettext function to the new method and from the global function call the method of the DefaultInstance like it is done e.g. in the GetText method.