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
1=== modified file 'configure.ac'
2--- configure.ac 2013-04-23 16:00:45 +0000
3+++ configure.ac 2013-04-24 18:28:26 +0000
4@@ -165,6 +165,8 @@
5 # Gettext
6 #############################################
7 IT_PROG_INTLTOOL([0.40.0])
8+GETTEXT_PACKAGE="$PACKAGE"
9+AC_SUBST(GETTEXT_PACKAGE)
10
11 #################################################
12 # Docs
13
14=== modified file 'protocol/Makefile.am'
15--- protocol/Makefile.am 2013-02-26 12:56:52 +0000
16+++ protocol/Makefile.am 2013-04-24 18:28:26 +0000
17@@ -31,6 +31,7 @@
18 libunity_protocol_private_la_CPPFLAGS = \
19 -DG_LOG_DOMAIN=\"libunity-protocol-private\" \
20 -DPKGDATADIR=\"$(PKGDATADIR)\" \
21+ -DGETTEXT_PACKAGE=\"$(GETTEXT_PACKAGE)\" \
22 -DDATADIR=\"$(DATADIR)\" \
23 -I$(srcdir) \
24 $(EXTRA_FLAGS) \
25
26=== modified file 'protocol/protocol-scope-discovery.vala'
27--- protocol/protocol-scope-discovery.vala 2013-03-26 11:37:24 +0000
28+++ protocol/protocol-scope-discovery.vala 2013-04-24 18:28:26 +0000
29@@ -114,10 +114,12 @@
30 public class ScopeRegistry
31 {
32 private static const string SCOPE_GROUP = "Scope";
33+ private static const string DESKTOP_GROUP = "Desktop Entry";
34
35 public class ScopeMetadata
36 {
37 public string id;
38+ public string domain;
39 public string full_path;
40 public string name;
41 public string dbus_path;
42@@ -140,8 +142,14 @@
43
44 public void load_from_key_file (KeyFile file) throws Error
45 {
46+ this.domain = null;
47+
48+ // Get the Gettext-Domain first, if it exists
49+ if (file.has_group (DESKTOP_GROUP) && file.has_key (DESKTOP_GROUP, "X-Ubuntu-Gettext-Domain"))
50+ this.domain = file.get_string (DESKTOP_GROUP, "X-Ubuntu-Gettext-Domain");
51+
52 // required fields
53- this.name = file.get_string (SCOPE_GROUP, "Name");
54+ this.name = dgettext(this.domain, file.get_string (SCOPE_GROUP, "Name"));
55 this.dbus_name = file.get_string (SCOPE_GROUP, "DBusName");
56 this.dbus_path = file.get_string (SCOPE_GROUP, "DBusPath");
57 this.icon = file.get_string (SCOPE_GROUP, "Icon");
58@@ -181,7 +189,7 @@
59 this.description = file.get_string (SCOPE_GROUP, "Description");
60
61 if (file.has_key (SCOPE_GROUP, "SearchHint"))
62- this.search_hint = file.get_string (SCOPE_GROUP, "SearchHint");
63+ this.search_hint = dgettext(this.domain, file.get_string (SCOPE_GROUP, "SearchHint"));
64
65 if (file.has_key (SCOPE_GROUP, "RequiredMetadata"))
66 this.required_metadata = MetaDataSchemaInfo.from_string (file.get_string (SCOPE_GROUP, "RequiredMetadata"));
67
68=== modified file 'test/data/unity/scopes/masterscope_b.scope'
69--- test/data/unity/scopes/masterscope_b.scope 2012-12-04 16:34:36 +0000
70+++ test/data/unity/scopes/masterscope_b.scope 2013-04-24 18:28:26 +0000
71@@ -12,4 +12,7 @@
72 Name=Sample Master Scope 2
73 Description=Find even more stuff
74 SearchHint=Search things
75-Shortcut=w
76\ No newline at end of file
77+Shortcut=w
78+
79+[Desktop Entry]
80+X-Ubuntu-Gettext-Domain=fakedomain
81
82=== modified file 'test/data/unity/scopes/masterscope_b/subscope1.scope'
83--- test/data/unity/scopes/masterscope_b/subscope1.scope 2012-12-04 16:34:36 +0000
84+++ test/data/unity/scopes/masterscope_b/subscope1.scope 2013-04-24 18:28:26 +0000
85@@ -10,4 +10,4 @@
86 Name=Sample subscope 1
87 Description=Find various stuff subscope 1
88 SearchHint=Search stuff subscope 1
89-Shortcut=x
90\ No newline at end of file
91+Shortcut=x
92
93=== modified file 'test/data/unity/scopes/masterscope_b/subscope2.scope'
94--- test/data/unity/scopes/masterscope_b/subscope2.scope 2012-12-04 16:34:36 +0000
95+++ test/data/unity/scopes/masterscope_b/subscope2.scope 2013-04-24 18:28:26 +0000
96@@ -8,4 +8,7 @@
97 Name=Sample subscope 2
98 Description=Find various stuff subscope 2
99 SearchHint=Search stuff subscope 2
100-Shortcut=u
101\ No newline at end of file
102+Shortcut=u
103+
104+[Desktop Entry]
105+X-Ubuntu-Gettext-Domain=fakedomain
106
107=== modified file 'test/data/unity/scopes/test_masterscope.scope'
108--- test/data/unity/scopes/test_masterscope.scope 2013-01-23 16:05:24 +0000
109+++ test/data/unity/scopes/test_masterscope.scope 2013-04-24 18:28:26 +0000
110@@ -11,3 +11,6 @@
111 Name=Sample Master Scope
112 Description=Find various stuff
113 SearchHint=Master search hint
114+
115+[Desktop Entry]
116+X-Ubuntu-Gettext-Domain=fakedomain
117
118=== modified file 'test/data/unity/scopes/test_masterscope/childscope_1.scope'
119--- test/data/unity/scopes/test_masterscope/childscope_1.scope 2012-12-21 12:55:38 +0000
120+++ test/data/unity/scopes/test_masterscope/childscope_1.scope 2013-04-24 18:28:26 +0000
121@@ -10,3 +10,6 @@
122 Name=Child scope 1
123 Description=Find various stuff
124 SearchHint=Search stuff
125+
126+[Desktop Entry]
127+X-Ubuntu-Gettext-Domain=fakedomain
128
129=== modified file 'test/data/unity/scopes/test_masterscope/childscope_2.scope'
130--- test/data/unity/scopes/test_masterscope/childscope_2.scope 2012-12-21 12:55:38 +0000
131+++ test/data/unity/scopes/test_masterscope/childscope_2.scope 2013-04-24 18:28:26 +0000
132@@ -10,3 +10,6 @@
133 Name=Child scope 2
134 Description=Find various stuff
135 SearchHint=Search stuff
136+
137+[Desktop Entry]
138+X-Ubuntu-Gettext-Domain=fakedomain

Subscribers

People subscribed via source and target branches