Merge lp:~townsend/libunity/fix-search-hint-localization into lp:~unity-team/libunity/libunity-7.0

Proposed by Christopher Townsend on 2013-04-01
Status: Merged
Approved by: Michal Hruby on 2013-04-24
Approved revision: 361
Merged at revision: 368
Proposed branch: lp:~townsend/libunity/fix-search-hint-localization
Merge into: lp:~unity-team/libunity/libunity-7.0
Diff against target: 138 lines (+31/-5)
9 files modified
configure.ac (+2/-0)
protocol/Makefile.am (+1/-0)
protocol/protocol-scope-discovery.vala (+10/-2)
test/data/unity/scopes/masterscope_b.scope (+4/-1)
test/data/unity/scopes/masterscope_b/subscope1.scope (+1/-1)
test/data/unity/scopes/masterscope_b/subscope2.scope (+4/-1)
test/data/unity/scopes/test_masterscope.scope (+3/-0)
test/data/unity/scopes/test_masterscope/childscope_1.scope (+3/-0)
test/data/unity/scopes/test_masterscope/childscope_2.scope (+3/-0)
To merge this branch: bzr merge lp:~townsend/libunity/fix-search-hint-localization
Reviewer Review Type Date Requested Status
Michal Hruby (community) 2013-04-01 Approve on 2013-04-24
PS Jenkins bot (community) continuous-integration Approve on 2013-04-24
Review via email: mp+156420@code.launchpad.net

Commit message

Add support to localize the initial search hint and home string.

Description of the change

The initial search hint in the Dash Search bar is not translated. This MP fixes the default search hint. A fix is also needed in unity-scope-home and also a unity-scope-home gettext domain is needed for the various different languages.

I tested this by first creating a unity-scope-home.mo file by coalescing the unity-lens-*.mo files by first running msgunfmt on those .mo files and converting them to .po files and then taking all of those and creating one big unity-scope-home.po file and then running msgfmt on that. I did this in Spanish since I can marginally read Spanish:)

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Christopher Townsend (townsend) wrote :

Looking at the Jenkins log, a test is failing that is not a result of this MP.

Christopher Townsend (townsend) wrote :

I'm wrong, the Jenkins failure is caused by this MP, but it's unclear why it would fail. Will investigate.

Christopher Townsend (townsend) wrote :

Ok, I see what the problem is now. The test masterscope_x.scope files do not contain the Desktop Entry part of the normal scope files and in turn, don't have X-Ubuntu-Gettext-Domain.

I will fix up these test scope files.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Michal Hruby (mhr3) wrote :

> Ok, I see what the problem is now. The test masterscope_x.scope files do not
> contain the Desktop Entry part of the normal scope files and in turn, don't
> have X-Ubuntu-Gettext-Domain.
>
> I will fix up these test scope files.

Sorry for taking so long to review this, ping us next time ;)
Wouldn't it be better to make the domain optional instead of failing if it's not there?

review: Needs Fixing
Christopher Townsend (townsend) wrote :

Hi Michal,

Ah, I forgot about this MP:) No worries!

Based on your review comment, are you saying that X-Ubuntu-Gettext-Domain would be an optional field in any .scope file? It's not clear to me if it should be, but it seems like it is always included in the .scope files and in the .lens files in 13.04. I assumed it was required based on 13.04 and seeing that all .scopes files in '100 scopes' have it included.

If it is required, then I think the code I have in place now should be used since it would be a failure if it's not there. If it's supposed to be optional, then yeah, I need to add some logic to handle the cases where it's not there.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Michal Hruby (mhr3) wrote :

> Hi Michal,
>
> Ah, I forgot about this MP:) No worries!
>
> Based on your review comment, are you saying that X-Ubuntu-Gettext-Domain
> would be an optional field in any .scope file? It's not clear to me if it
> should be, but it seems like it is always included in the .scope files and in
> the .lens files in 13.04. I assumed it was required based on 13.04 and seeing
> that all .scopes files in '100 scopes' have it included.
>

Yes, conceptually it's a .desktop file key in a scope file, so it's quite likely that we'll want to change that at some point, therefore it shouldn't be a required field.

> If it is required, then I think the code I have in place now should be used
> since it would be a failure if it's not there. If it's supposed to be
> optional, then yeah, I need to add some logic to handle the cases where it's
> not there.

Yes, please adjust the logic to make it not fail if the key is not there (plus revert at least one test .scope file to also test that possibility.

Christopher Townsend (townsend) wrote :

Ok, I added logic to check if the "Desktop Entry" group and the X-Ubuntu-Gettext-Domain key exist. Also removed those entries in a couple of test scope files to make sure that the logic works for both cases.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Michal Hruby (mhr3) wrote :

8 +#############################################
9 +# Gettext
10 +#############################################
11 +GETTEXT_PACKAGE="..."
12 +AC_SUBST(GETTEXT_PACKAGE)
13 +

There's already a Gettext section, could you move it there pls?

review: Needs Fixing
360. By Christopher Townsend on 2013-04-24

A previous commit added an unnecessary Gettext section. Move the added configuration vars to the already existing Gettext section.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
361. By Christopher Townsend on 2013-04-24

Using "..." for the GETTEXT_PACKAGE is not advised. Use $PACKAGE for that instead.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Michal Hruby (mhr3) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'configure.ac'
--- configure.ac 2013-04-23 16:00:45 +0000
+++ configure.ac 2013-04-24 18:28:26 +0000
@@ -165,6 +165,8 @@
165# Gettext165# Gettext
166#############################################166#############################################
167IT_PROG_INTLTOOL([0.40.0])167IT_PROG_INTLTOOL([0.40.0])
168GETTEXT_PACKAGE="$PACKAGE"
169AC_SUBST(GETTEXT_PACKAGE)
168170
169#################################################171#################################################
170# Docs172# Docs
171173
=== modified file 'protocol/Makefile.am'
--- protocol/Makefile.am 2013-02-26 12:56:52 +0000
+++ protocol/Makefile.am 2013-04-24 18:28:26 +0000
@@ -31,6 +31,7 @@
31libunity_protocol_private_la_CPPFLAGS = \31libunity_protocol_private_la_CPPFLAGS = \
32 -DG_LOG_DOMAIN=\"libunity-protocol-private\" \32 -DG_LOG_DOMAIN=\"libunity-protocol-private\" \
33 -DPKGDATADIR=\"$(PKGDATADIR)\" \33 -DPKGDATADIR=\"$(PKGDATADIR)\" \
34 -DGETTEXT_PACKAGE=\"$(GETTEXT_PACKAGE)\" \
34 -DDATADIR=\"$(DATADIR)\" \35 -DDATADIR=\"$(DATADIR)\" \
35 -I$(srcdir) \36 -I$(srcdir) \
36 $(EXTRA_FLAGS) \37 $(EXTRA_FLAGS) \
3738
=== modified file 'protocol/protocol-scope-discovery.vala'
--- protocol/protocol-scope-discovery.vala 2013-03-26 11:37:24 +0000
+++ protocol/protocol-scope-discovery.vala 2013-04-24 18:28:26 +0000
@@ -114,10 +114,12 @@
114 public class ScopeRegistry114 public class ScopeRegistry
115 {115 {
116 private static const string SCOPE_GROUP = "Scope";116 private static const string SCOPE_GROUP = "Scope";
117 private static const string DESKTOP_GROUP = "Desktop Entry";
117118
118 public class ScopeMetadata119 public class ScopeMetadata
119 {120 {
120 public string id;121 public string id;
122 public string domain;
121 public string full_path;123 public string full_path;
122 public string name;124 public string name;
123 public string dbus_path;125 public string dbus_path;
@@ -140,8 +142,14 @@
140142
141 public void load_from_key_file (KeyFile file) throws Error143 public void load_from_key_file (KeyFile file) throws Error
142 {144 {
145 this.domain = null;
146
147 // Get the Gettext-Domain first, if it exists
148 if (file.has_group (DESKTOP_GROUP) && file.has_key (DESKTOP_GROUP, "X-Ubuntu-Gettext-Domain"))
149 this.domain = file.get_string (DESKTOP_GROUP, "X-Ubuntu-Gettext-Domain");
150
143 // required fields151 // required fields
144 this.name = file.get_string (SCOPE_GROUP, "Name");152 this.name = dgettext(this.domain, file.get_string (SCOPE_GROUP, "Name"));
145 this.dbus_name = file.get_string (SCOPE_GROUP, "DBusName");153 this.dbus_name = file.get_string (SCOPE_GROUP, "DBusName");
146 this.dbus_path = file.get_string (SCOPE_GROUP, "DBusPath");154 this.dbus_path = file.get_string (SCOPE_GROUP, "DBusPath");
147 this.icon = file.get_string (SCOPE_GROUP, "Icon");155 this.icon = file.get_string (SCOPE_GROUP, "Icon");
@@ -181,7 +189,7 @@
181 this.description = file.get_string (SCOPE_GROUP, "Description");189 this.description = file.get_string (SCOPE_GROUP, "Description");
182190
183 if (file.has_key (SCOPE_GROUP, "SearchHint"))191 if (file.has_key (SCOPE_GROUP, "SearchHint"))
184 this.search_hint = file.get_string (SCOPE_GROUP, "SearchHint");192 this.search_hint = dgettext(this.domain, file.get_string (SCOPE_GROUP, "SearchHint"));
185193
186 if (file.has_key (SCOPE_GROUP, "RequiredMetadata"))194 if (file.has_key (SCOPE_GROUP, "RequiredMetadata"))
187 this.required_metadata = MetaDataSchemaInfo.from_string (file.get_string (SCOPE_GROUP, "RequiredMetadata"));195 this.required_metadata = MetaDataSchemaInfo.from_string (file.get_string (SCOPE_GROUP, "RequiredMetadata"));
188196
=== modified file 'test/data/unity/scopes/masterscope_b.scope'
--- test/data/unity/scopes/masterscope_b.scope 2012-12-04 16:34:36 +0000
+++ test/data/unity/scopes/masterscope_b.scope 2013-04-24 18:28:26 +0000
@@ -12,4 +12,7 @@
12Name=Sample Master Scope 212Name=Sample Master Scope 2
13Description=Find even more stuff13Description=Find even more stuff
14SearchHint=Search things14SearchHint=Search things
15Shortcut=w
16\ No newline at end of file15\ No newline at end of file
16Shortcut=w
17
18[Desktop Entry]
19X-Ubuntu-Gettext-Domain=fakedomain
1720
=== modified file 'test/data/unity/scopes/masterscope_b/subscope1.scope'
--- test/data/unity/scopes/masterscope_b/subscope1.scope 2012-12-04 16:34:36 +0000
+++ test/data/unity/scopes/masterscope_b/subscope1.scope 2013-04-24 18:28:26 +0000
@@ -10,4 +10,4 @@
10Name=Sample subscope 110Name=Sample subscope 1
11Description=Find various stuff subscope 111Description=Find various stuff subscope 1
12SearchHint=Search stuff subscope 112SearchHint=Search stuff subscope 1
13Shortcut=x
14\ No newline at end of file13\ No newline at end of file
14Shortcut=x
1515
=== modified file 'test/data/unity/scopes/masterscope_b/subscope2.scope'
--- test/data/unity/scopes/masterscope_b/subscope2.scope 2012-12-04 16:34:36 +0000
+++ test/data/unity/scopes/masterscope_b/subscope2.scope 2013-04-24 18:28:26 +0000
@@ -8,4 +8,7 @@
8Name=Sample subscope 28Name=Sample subscope 2
9Description=Find various stuff subscope 29Description=Find various stuff subscope 2
10SearchHint=Search stuff subscope 210SearchHint=Search stuff subscope 2
11Shortcut=u
12\ No newline at end of file11\ No newline at end of file
12Shortcut=u
13
14[Desktop Entry]
15X-Ubuntu-Gettext-Domain=fakedomain
1316
=== modified file 'test/data/unity/scopes/test_masterscope.scope'
--- test/data/unity/scopes/test_masterscope.scope 2013-01-23 16:05:24 +0000
+++ test/data/unity/scopes/test_masterscope.scope 2013-04-24 18:28:26 +0000
@@ -11,3 +11,6 @@
11Name=Sample Master Scope11Name=Sample Master Scope
12Description=Find various stuff12Description=Find various stuff
13SearchHint=Master search hint13SearchHint=Master search hint
14
15[Desktop Entry]
16X-Ubuntu-Gettext-Domain=fakedomain
1417
=== modified file 'test/data/unity/scopes/test_masterscope/childscope_1.scope'
--- test/data/unity/scopes/test_masterscope/childscope_1.scope 2012-12-21 12:55:38 +0000
+++ test/data/unity/scopes/test_masterscope/childscope_1.scope 2013-04-24 18:28:26 +0000
@@ -10,3 +10,6 @@
10Name=Child scope 110Name=Child scope 1
11Description=Find various stuff11Description=Find various stuff
12SearchHint=Search stuff12SearchHint=Search stuff
13
14[Desktop Entry]
15X-Ubuntu-Gettext-Domain=fakedomain
1316
=== modified file 'test/data/unity/scopes/test_masterscope/childscope_2.scope'
--- test/data/unity/scopes/test_masterscope/childscope_2.scope 2012-12-21 12:55:38 +0000
+++ test/data/unity/scopes/test_masterscope/childscope_2.scope 2013-04-24 18:28:26 +0000
@@ -10,3 +10,6 @@
10Name=Child scope 210Name=Child scope 2
11Description=Find various stuff11Description=Find various stuff
12SearchHint=Search stuff12SearchHint=Search stuff
13
14[Desktop Entry]
15X-Ubuntu-Gettext-Domain=fakedomain

Subscribers

People subscribed via source and target branches