Opened 9 years ago

Closed 9 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 by stippi, 9 years ago

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 by Karvjorm, 9 years ago

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 by nielx, 9 years ago

Has a Patch: set

comment:4 by stippi, 9 years ago

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

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

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 by stippi, 9 years ago

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.

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

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 by pulkomandy, 9 years ago

Owner: changed from nobody to pulkomandy
Status: newassigned

comment:9 by pulkomandy, 9 years ago

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

comment:10 by pulkomandy, 9 years ago

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 !

by Karvjorm, 9 years ago

Attachment: devicesLocalization.patch added

Third devices localization patch

comment:11 by pulkomandy, 9 years ago

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 by siarzhuk, 9 years ago

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

comment:13 by siarzhuk, 9 years ago

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.