Opened 8 years ago

Closed 8 years ago

#7169 closed enhancement (fixed)

A DiskUsage localization patch

Reported by: Karvjorm Owned by: siarzhuk
Priority: normal Milestone: R1
Component: Applications/DiskUsage Version: R1/alpha2
Keywords: DiskUsage localization patch Cc: Karvjorm
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

Here is a DiskUsage localization patch.

Attachments (2)

diskusage-localization.patch (5.3 KB) - added by Karvjorm 8 years ago.
A DiskUsage localization patch
diskusage-refactoring.patch (20.1 KB) - added by Karvjorm 8 years ago.
A DiskUsage localization refactoring patch

Download all attachments as: .zip

Change History (8)

Changed 8 years ago by Karvjorm

A DiskUsage localization patch

comment:1 Changed 8 years ago by Karvjorm

Has a Patch: set

comment:2 Changed 8 years ago by siarzhuk

Owner: changed from nobody to siarzhuk
Status: newassigned

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. ;-)

comment:3 in reply to:  2 ; Changed 8 years ago by 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?

comment:4 in reply to:  3 Changed 8 years ago by 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.

Changed 8 years ago by Karvjorm

Attachment: diskusage-refactoring.patch added

A DiskUsage localization refactoring patch

comment:5 Changed 8 years ago by 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

comment:6 Changed 8 years ago by siarzhuk

Resolution: fixed
Status: assignedclosed

Applied in hrev40493. Thanks.

Note: See TracTickets for help on using tickets.