Opened 9 years ago

Closed 8 years ago

#5912 closed enhancement (fixed)

Devices localization patch

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

Description

Here is a localization patch for Devices application.

Attachments (1)

devicesLocalization.patch (17.0 KB) - added by Karvjorm 9 years ago.
Third devices localization patch

Download all attachments as: .zip

Change History (14)

comment:1 Changed 9 years ago by stippi

Patch looks mostly good. I am wondering about this part:

+ private:
+    const char* fUnknown = TR("unknown"); 

Are you sure this works? Does it even compile?

comment:2 Changed 9 years ago by Karvjorm

I have usually compiled code like "g++ -c -I/Blank_BFS/haiku/src/apps/devices -I/Blank_BFS/haiku/src/apps... main.cpp" for each .cpp file, but now I do not remember, if I completed it for these files.

comment:3 Changed 9 years ago by nielx

Has a Patch: set

comment:4 Changed 9 years ago by stippi

Karvjorm, if I am not mistaken, you should have a working build environemt now? Can you revisit my comment above? Thanks and sorry for the delays in looking at your patches...

comment:5 in reply to:  4 Changed 9 years ago by Karvjorm

Replying to stippi:

Karvjorm, if I am not mistaken, you should have a working build environemt now? Can you revisit my comment above? Thanks and sorry for the delays in looking at your patches...

No it is still broken. I can build other things but it fails always when libroot.so and libbe.so are needed. I have not time to study it more just now (maybe tonight or tomorrow next time), but I have planned to test it with the latest nightly build berofe r1alpha2.

comment:6 Changed 9 years ago by stippi

Why is there one fAppCatalog per translated file, additionally to the one allocated in main()? The patch also still contains translation-unfriendly string composition:

@@ -102,17 +132,21 @@
 Device::GetBasicStrings()
 {
 	BString str;
-	str << "Device Name\t\t\t\t: " << GetName() << "\n";
-	str << "Manufacturer\t\t\t: " << GetManufacturer() << "\n";
-	str << "Driver used\t\t\t\t: " << GetDriverUsed() << "\n";
-	str << "Device paths\t: " << GetDevPathsPublished();
+	str << B_TRANSLATE("Device Name");
+	str << "\t\t\t\t: " << GetName() << "\n";
+	str << B_TRANSLATE("Manufacturer");
+	str << "\t\t\t: " << GetManufacturer() << "\n";
+	str << B_TRANSLATE("Driver used");
+	str << "\t\t\t\t: " << GetDriverUsed() << "\n";
+	str << B_TRANSLATE("Device paths");
+	str << "\t: " << GetDevPathsPublished();
 	return str;
 }

This should be more like

    BString string = B_TRANSLATE("Device Name\t\t\t\t: %1\n"
        "Manufacturer\t\t\t: %2\n"
        "Driver used\t\t\t\t: %3\n"
        "Device paths\t: %4\n");
    string.ReplaceFirst("%1", GetName());
    string.ReplaceFirst("%2", GetManufacturer());
    string.ReplaceFirst("%3", GetDriverUsed());
    string.ReplaceFirst("%4", GetDevPathsPublished());
    return string;

This way, a translation can change the order (think bottom to top languages) and the number of tabs can be adjusted for the actual string widths, so that the alignment can be preserved.

comment:7 in reply to:  6 Changed 9 years ago by Karvjorm

Replying to stippi:

Why is there one fAppCatalog per translated file, additionally to the one allocated in main()?

It is my mistake. Surely there could be only one BCatalog and initialization. I did not think clearly there.

The patch also still contains translation-unfriendly string composition:

This should be more like

    BString string = B_TRANSLATE("Device Name\t\t\t\t: %1\n"
        "Manufacturer\t\t\t: %2\n"
        "Driver used\t\t\t\t: %3\n"
         "Device paths\t: %4\n");
    string.ReplaceFirst("%1", GetName());
    string.ReplaceFirst("%2", GetManufacturer());
    string.ReplaceFirst("%3", GetDriverUsed());
    string.ReplaceFirst("%4", GetDevPathsPublished());
    return string;

This way, a translation can change the order (think bottom to top languages) and the number of tabs can be adjusted for the actual string widths, so that the alignment can be preserved.

Thank you. It was just I said that I'm an ordinary programmer, not a guru. You can always find several solutions but gurus find better solutions and faster. ;)

comment:8 Changed 9 years ago by pulkomandy

Owner: changed from nobody to pulkomandy
Status: newassigned

comment:9 Changed 9 years ago by pulkomandy

I'm still waitig on a cleanup for this one :)

comment:10 Changed 9 years ago by pulkomandy

Looks mostly fine, but there are still some details...

  • Skip two lines between blocks (including #define\n\n#include and }\n\n#undef TR_CONTEXT)
  • In some places you added a space after :. That's not style-compliant
  • You don't need to link "locale" in the Jamfile, only $(HAIKU_LOCALE_LIBS).

Thanks for your efforts anyway !

Changed 9 years ago by Karvjorm

Attachment: devicesLocalization.patch added

Third devices localization patch

comment:11 Changed 9 years ago by pulkomandy

There are still problems with the blank lines between blocks.

You have to skip two lines between the last #include statement and the #undef TR_CONTEXT, and two lines again after the #define TR_CONTEXT and before the start of the code. It should look like this :

...
#include <something.h>


#undef TR_CONTEXT
#define TR_CONTEXT my_app


class SomeThing {
...

I see you also added some logic in device.cpp to handle the "unknown" case. I think it is simpler to localize the string directly in Devices.h, so that you don't have to mess with it afterwards. Or does that create other problems ?

Also, the way you added HAIKU_LOCALE_LIBS in the jamfile goes over the 80-column limit.

comment:12 Changed 8 years ago by siarzhuk

Jorma, please check the actual state of Devices application localization and provide actual patch if is is still required.

comment:13 Changed 8 years ago by siarzhuk

Resolution: fixed
Status: assignedclosed

Looks like obsoleted by #7111. Feel free to reopen it and any fixes to it.

Note: See TracTickets for help on using tickets.