Opened 13 years ago

Closed 3 years ago

Last modified 3 years ago

#7530 closed enhancement (fixed)

[Cortex] Cortex is not localized

Reported by: deejam Owned by: nielx
Priority: normal Milestone: R1/beta3
Component: Applications/Cortex Version: R1/Development
Keywords: localization Cc: nielx
Blocked By: Blocking:
Platform: All

Description

Cortex is not localized.

r1a3-rc-hrev41550

Attachments (5)

Cortex-not-localized.png (30.2 KB ) - added by deejam 13 years ago.
0001-Added-localization-support-for-Cortex-app.patch (115.1 KB ) - added by dsjonny 11 years ago.
new-0001-Added-localization-support-for-Cortex-app.patch (115.2 KB ) - added by humdinger 11 years ago.
updated patch
screenshot1.png (43.2 KB ) - added by bitigchi 3 years ago.
screenshot
screenshot-new.png (276.7 KB ) - added by bitigchi 3 years ago.
screenshot-new

Download all attachments as: .zip

Change History (31)

by deejam, 13 years ago

Attachment: Cortex-not-localized.png added

comment:1 by mmadia, 11 years ago

Component: ApplicationsApplications/Cortex

comment:2 by dsjonny, 11 years ago

I am on the way to add localization support to Cortext, but there are many files with many strings, so I need more 1-2 days (about 40% of translation added...)...

comment:3 by dsjonny, 11 years ago

patch: 01

comment:4 by dsjonny, 11 years ago

I have attached a patch for the localization support for Cortex. Please check it, I'm not sure it is 100% correct.

comment:5 by diver, 11 years ago

Thanks for your work! Could you please correct case in GUI strings like it was done here?

comment:6 by dsjonny, 11 years ago

Patch has been replaced. I have changed the strings to lowercase.

comment:7 by humdinger, 11 years ago

Owner: changed from nobody to humdinger
Status: newassigned

I'm having a look at it.

comment:8 by humdinger, 11 years ago

The patch doesn't apply cleanly. I did a few minor changes and uploaded a new version of your patch. Mostly it was left over sentence case issues, plus removed spaces around strings that were arguments for "StringWidth()". Those might be better added with "+ " ""(?).

Anyway, it doesn't compile. jam complains "don't know how to make be" and "...can't find 1 target(s)..." and "...can't make 3 target(s)...".

It seems not not like this in a Jamfile (here of DormantNode):

 	DormantNodeListItem.cpp
 	DormantNodeView.cpp
 	DormantNodeWindow.cpp
	: be $(HAIKU_LOCALE_LIBS)

I'm not a jam expert, so I can't propose a solution... (therefore I unclaim this ticket for now)

In general, this localization is a good first step. There are however a few places where we find things like:

	BString s;
-	s << "Could not instantiate '" << info.name << "'";
+	s << B_TRANSLATE("Could not instantiate") << " '" << info.name << "'";

which should be rewritten using a variable that is then replaced with ReplaceFirst().

comment:9 by humdinger, 11 years ago

Owner: changed from humdinger to nobody

by humdinger, 11 years ago

updated patch

comment:10 by mmadia, 11 years ago

Owner: changed from nobody to pulkomandy

Adrien, would you be interested in looking over this patch?

comment:11 by pulkomandy, 11 years ago

Quick look at the patch:

64	 	    setSideBarWidth(be_plain_font->StringWidth(" Destination ")
 	69	    setSideBarWidth(be_plain_font->StringWidth(B_TRANSLATE("Destination")) 

This makes the view narrower than before. Does it still look ok ? There's already a margin added to it, but maybe it needs to be increased. There are other places where the same thing happens again.

141	 	            setSideBarWidth(be_plain_font->StringWidth(" Video Data Between ") + 2 * InfoView::M_H_MARGIN);
 	146	            setSideBarWidth(be_plain_font->StringWidth(B_TRANSLATE(" Video data between ")) + 2 * InfoView::M_H_MARGIN); 

You removed the spaces in other cases, why keep them here ? Try to be consistent. (remove the spaces everywhere, and adjust the margins, or possibly add the spaces after localisation)

118	 	            addField("Format", s);
 	123	            addField(B_TRANSLATE("Format"), s); 

(and all addField calls): it seems this method will do something like arg1 + ": " + arg2. This is not locale safe.

120	 	                            s << "\n- Codec: " << codec.pretty_name;
 	124	                            s << B_TRANSLATE("\n- Codec: ") << codec.pretty_name;
121	125	                            if (codec.id > 0)
122	126	                            {
123	 	                                s << " (ID: " << codec.id << ")";
 	127	                                s << B_TRANSLATE(" (ID: ") << codec.id << ")";
124	128	                            } 
179	 	    title << " info";
 	184	    title << B_TRANSLATE(" info"); 

(and some other places)

Do not use the << operator this way. The translator should see a string like this: 'Codec: %codec% (ID: %id%)" with a comment saying that the ID part may be removed. Use BString.ReplaceFirst to replace the %% tags with the proper values, after calling B_TRANSLATE.

 	414	const char* const           NodeManager::s_defaultGroupPrefix = B_TRANSLATE("No name");
 	415	const char* const           NodeManager::s_timeSourceGroup = B_TRANSLATE("Time sources");
 	416	const char* const           NodeManager::s_audioInputGroup = B_TRANSLATE("System audio input");
 	417	const char* const           NodeManager::s_videoInputGroup = B_TRANSLATE("System video input");
 	418	const char* const           NodeManager::s_audioMixerGroup = B_TRANSLATE("System audio mixer");
 	419	const char* const           NodeManager::s_videoOutputGroup = B_TRANSLATE("System video output"); 

You can't use B_TRANSLATE on const char* const, since the string is not a constant anymore. You have to use B_TRANSLATE_MARK to tag the string for collectcatkeys, then use B_TRANSLATE on the constant, everywhere it is actually used. This does not even compile, so I suspect the patch wasn't tested ?

 	92	        sprintf(title, B_TRANSLATE("%s parameters"), nodeInfo.name); 

Same as the << operator, use ReplaceFirst instead, or use B_TRANSLATE_COMMENT to inform the translator what the %s is replaced with. When there are multiple % escapes in a string used in printf family function, switch to ReplaceFirst, because the tokens may be in a different order in the translated string and that will confuse printf.

You added HAIKU_LOCALE_LIBS to each .a archive built. You only need it once for the application. You also added a separate DoCatalogs invocation for each of the add-ons. I'm not sure that's the right thing to do. We could have the add-on load their strings from the application catalog, since they can't be used anywhere else (so there are less catalog files). That requires some additional jam and C++ tricks, however.

comment:12 by dsjonny, 11 years ago

I'm sorry, but I forgot to write next to the patch: it was not tested, because I could not compile it (that's why I wrote: please check it). I will try to make the corections what you wrote above. Thank you for the help!

comment:13 by diver, 4 years ago

We should move this patch to Gerrit.

comment:14 by diver, 4 years ago

Keywords: localization added

comment:16 by pulkomandy, 3 years ago

Milestone: R1R1/beta3
Resolution: fixed
Status: assignedclosed

Patch merged in hrev55011.

comment:17 by bitigchi, 3 years ago

It looks like the strings didn't make it to Pootle.

comment:18 by pulkomandy, 3 years ago

Cc: nielx added

Hi nielx, do you know if there is something to do at Pootle to add new apps/libraries?

Last edited 3 years ago by pulkomandy (previous) (diff)

comment:19 by nielx, 3 years ago

I see that they're generated, but they are not copied into the various languages. It looks like that's a gap in the scripts to propagate these changes. In any case, they are there now, ready for translation.

by bitigchi, 3 years ago

Attachment: screenshot1.png added

screenshot

comment:20 by bitigchi, 3 years ago

It looks like it's still not complete. Main application menus and some other parts are still not localised, although the strings have been translated.

comment:21 by humdinger, 3 years ago

Resolution: fixed
Status: closedreopened

comment:22 by nielx, 3 years ago

Owner: changed from pulkomandy to nielx
Status: reopenedin-progress

comment:24 by nielx, 3 years ago

Resolution: fixed
Status: in-progressclosed

by bitigchi, 3 years ago

Attachment: screenshot-new.png added

screenshot-new

comment:25 by bitigchi, 3 years ago

It looks like there are still some untranslated strings on the main window. I am not sure if the extensions are supposed to be translated but untranslated nodes surely look weird.

in reply to:  25 comment:26 by nielx, 3 years ago

Replying to bitigchi:

It looks like there are still some untranslated strings on the main window. I am not sure if the extensions are supposed to be translated but untranslated nodes surely look weird.

Could you open a new ticket for these leftover translations?

Note: See TracTickets for help on using tickets.