A DiskUsage localization patch

[Karvjorm]

Here is a DiskUsage localization patch.

[Karvjorm]

A DiskUsage localization patch

[siarzhuk]

From my point of view, this patch is unacceptable. The problem is a way the DiskUsage application work with strings. Typical Haiku applications, as you know, use string literals in the source code. But DiskUsage uses string resources to store this info. Look into DiskUsage.rdef for details. Frankly speaking the work with resources in Haiku is not so perfect as other parts of it. Another possible problem is a memory leaks - I see no free calls for loaded string resources. And last problem - is the way you trying to workaround the translation task:

 	kVolMenuLabel = LoadString("STR_VM_LABEL");
+	if (strncmp(kVolMenuLabel, "Volume:", sizeof("Volume:")) == 0)
+		kVolMenuLabel = (char *)B_TRANSLATE("Volume:");
+

it looks like an OVER-complication. :-(

So I propose you to move away mentioned string resources from DiskUsage.rdef, and replace corresponding calls of "LoadString" in Common.cpp with those strings wrapped in B_TRANSLATE macros.

PS: In my opinion, "color" and "int" can be also moved from DiskUsage.rdef to Common.cpp. It makes the code a bit more simpler. ;-)

[Karvjorm]

Replying to siarzhuk:

From my point of view, this patch is unacceptable. The problem is a way the DiskUsage application work with strings. Typical Haiku applications, as you know, use string literals in the source code. But DiskUsage uses string resources to store this info. Look into DiskUsage.rdef for details. Frankly speaking the work with resources in Haiku is not so perfect as other parts of it. Another possible problem is a memory leaks - I see no free calls for loaded string resources. And last problem - is the way you trying to workaround the translation task:

 	kVolMenuLabel = LoadString("STR_VM_LABEL");
+	if (strncmp(kVolMenuLabel, "Volume:", sizeof("Volume:")) == 0)
+		kVolMenuLabel = (char *)B_TRANSLATE("Volume:");
+

it looks like an OVER-complication. :-(

So I propose you to move away mentioned string resources from DiskUsage.rdef, and replace corresponding calls of "LoadString" in Common.cpp with those strings wrapped in B_TRANSLATE macros.

PS: In my opinion, "color" and "int" can be also moved from DiskUsage.rdef to Common.cpp. It makes the code a bit more simpler. ;-)

OK, I tried to do the localization work without large refactoring work and noticed that those strings that are target for the localization were loaded from the resource file. So I decided to localize those dynamically on-the-fly after there were loaded to the application. I did not know any other way (except refactoring the code and inserting strings to source code and ignoring strings in the resource file). But maybe I have to do it then?

[siarzhuk]

Replying to Karvjorm:

Replying to siarzhuk: OK, I tried to do the localization work without large refactoring work and noticed that those strings that are target for the localization were loaded from the resource file. So I decided to localize those dynamically on-the-fly after there were loaded to the application. I did not know any other way (except refactoring the code and inserting strings to source code and ignoring strings in the resource file). But maybe I have to do it then?

Yes, do it. Just grep through the source for every name of those "quasi"-constant loaded from resource and replace it directly with translated string literal. Anyway our catalogs are already some kind of resources. ;-) And the code will be more clear and compact. Corresponding strings from DiskUsage.rdef and any references should be, of course, purged.

[Karvjorm]

A DiskUsage localization refactoring patch

[siarzhuk]

OK, I'll check and apply it in some days. Thank you!

BTW, I found it a bit unsafe to let the translator change the following mask:

 strftime(name, 64, B_TRANSLATE("%a, %d %b %Y, %r"), localtime(&t));

May be a NOTE: "If you don't know the strftime format - do not touch it!" must be added here? From other side we let them change the printf-like masks in many other places and we are still alive, so, probably it is also not more dangerous. :-D

[siarzhuk]

Applied in hrev40493. Thanks.