After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 787105 - Migrated from Intltool to Gettext
Migrated from Intltool to Gettext
Status: RESOLVED FIXED
Product: gnome-photos
Classification: Applications
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: GNOME photos maintainer(s)
GNOME photos maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-08-31 20:14 UTC by Iñigo Martínez
Modified: 2017-09-05 06:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Migrated from Intltool to Gettext (8.21 KB, patch)
2017-08-31 20:14 UTC, Iñigo Martínez
none Details | Review
Migrated from Intltool to Gettext (14.16 KB, patch)
2017-09-01 10:59 UTC, Iñigo Martínez
none Details | Review
Migrated from Intltool to Gettext (11.92 KB, patch)
2017-09-01 16:03 UTC, Iñigo Martínez
none Details | Review
Migrated from Intltool to Gettext (12.17 KB, patch)
2017-09-03 14:20 UTC, Iñigo Martínez
none Details | Review
Migrate from Intltool to Gettext (11.98 KB, patch)
2017-09-04 17:30 UTC, Debarshi Ray
committed Details | Review
org.gnome.Photos.desktop: Add translator comments (1.11 KB, patch)
2017-09-04 17:30 UTC, Debarshi Ray
committed Details | Review

Description Iñigo Martínez 2017-08-31 20:14:12 UTC
Created attachment 358888 [details] [review]
Migrated from Intltool to Gettext

This patch migrates translations from intltool to gettext. It depends on meson build port patch as it uses meson's i18n module.

Although almost all translatable strings were extracted, the default AppData rules don't extract the 'developer_name' tag, so a new extended ITS rules have also been created.
Comment 1 Debarshi Ray 2017-08-31 21:30:36 UTC
Review of attachment 358888 [details] [review]:

Thanks for the patch! I was hoping to get rid of intltool first before porting to Meson. Removing intltool is simpler, and if we retain the Autotools build we can continue to do so without relying on intltool. You could use https://git.gnome.org/browse/gnome-documents/commit/?id=4df587ed389359d273aa236112a1c035702ca5a1 as an example.
Comment 2 Debarshi Ray 2017-08-31 21:34:51 UTC
Oh, and thanks for noticing the problem with <developer_name> not being translated by the default gettext rules! In fact I was sloppy and forgot to remove the intltool underscore marker when porting away from it. I guess we should add the same to gnome-documents' Autotools build.

I swear I checked the POT files. No idea how I missed it. :(
Comment 3 Piotr Drąg 2017-08-31 21:52:24 UTC
its rules from both AppStream (https://github.com/ximion/appstream/blob/master/data/its/metainfo.its) and AppStream-GLib (https://github.com/hughsie/appstream-glib/blob/master/data/appdata.its) already extract <developer_name>, and so will gettext in a future version (https://savannah.gnu.org/bugs/?51881). Damned-Lies is configured to use AppStream’s (https://git.gnome.org/browse/damned-lies/tree/damnedlies/settings.py#n163).
Comment 4 Iñigo Martínez 2017-09-01 10:59:17 UTC
Created attachment 358924 [details] [review]
Migrated from Intltool to Gettext

I updated the patch with a different approach. This migrates autotools from intltool to gettext.

I have also included the 'appdata.[its|loc]' files for extracting and translating the 'developer_name' tag until appstream and/or gettext are released.

Although translated strings are merged properly into AppData and Desktop files, including the 'developer_name' tag, string extraction is a different problem I have not been able to figure out properly yet. I have moved the ITS rules files to the data directory and I have also created an script which extracts all the strings from the AppData file, by following what gtksourceview does.

Despite the fact that this 'works', I'm not very happy as it does generate two split 'pot' files instead of a centralized pot file. This could be solved somehow by "intercepting" 'xgettext' and wrapping it with 'msgcat', which can merge both 'pot' files into one centralized pot file.

However, I'm not sure how this should be done, and I'm also not sure if this is worth enough.

Finally, this patch 'invalidates' the meson build port patch, that should rebase this.

[0] https://git.gnome.org/browse/gtksourceview/tree/data/glade
Comment 5 Piotr Drąg 2017-09-01 15:11:35 UTC
This its/loc dance really shouldn’t be necessary. Are you sure you have “appstream” package installed? It provides /usr/share/gettext/its/metainfo.[its|loc] — at least on Fedora.
Comment 6 Iñigo Martínez 2017-09-01 16:03:31 UTC
Created attachment 358947 [details] [review]
Migrated from Intltool to Gettext

Oh, my bad! I didn't have 'appstream' installed. I was using ITS rules from gettext-0.19.8.

After installing it, it worked fine without any workaround, so I updated the patch with complete gettext support.

Thank you Piotr Drąg.
Comment 7 Debarshi Ray 2017-09-03 12:43:55 UTC
Review of attachment 358947 [details] [review]:

Thanks for updating the patch, Iñigo. I only took a quick look because I don't have the time to actually build and test it, but I couldn't spot anything wrong. I'll test it and include it in the 3.25.92 tarball.

Piotr, if you could indulge me once more, could you confirm that the gettext setup in gnome-documents is fine? I am only asking because I tend to use it as a frame of reference. :)
Comment 8 Piotr Drąg 2017-09-03 13:35:20 UTC
I didn’t test it, but it looks fine. I only added these comments: https://git.gnome.org/browse/gnome-documents/commit/?id=3d0fd78b9a812b66e6479585c1a2d1ae8092fb94 (because of https://savannah.gnu.org/support/?108887).
Comment 9 Iñigo Martínez 2017-09-03 14:20:23 UTC
Created attachment 359025 [details] [review]
Migrated from Intltool to Gettext

I have updated the patch to include those comments.
Comment 10 Debarshi Ray 2017-09-04 17:30:10 UTC
Review of attachment 359025 [details] [review]:

Would be nice to add the bug URL to the commit message. :)

A couple of minor niggles:

::: Makefile.am
@@ +1,3 @@
 ACLOCAL_AMFLAGS = -I m4 -I libgd ${ACLOCAL_FLAGS}
 
+SUBDIRS = . po data libgd src tests help

Umm... why is this necessary? According to https://wiki.gnome.org/MigratingFromIntltoolToGettext the .desktop file should be built before the po sub-directory.

::: data/org.gnome.Photos.desktop.in.in
@@ +4,2 @@
 Exec=@PACKAGE_TARNAME@
+# Translators: Do NOT translate or transliterate this text (this is an icon file name)!

These comments should be added in a separate patch.
Comment 11 Debarshi Ray 2017-09-04 17:30:44 UTC
Created attachment 359093 [details] [review]
Migrate from Intltool to Gettext
Comment 12 Debarshi Ray 2017-09-04 17:30:58 UTC
Created attachment 359094 [details] [review]
org.gnome.Photos.desktop: Add translator comments
Comment 13 Debarshi Ray 2017-09-04 17:32:23 UTC
I took the liberty to adjust the patch(es). I compared the .pot file before and after the patches, and couldn't find any delta other than the Icon string getting spuriously picked up due to https://savannah.gnu.org/support/?108887

If you don't have any objections, then I'll merge these and spin the 3.25.92 tarball tomorrow.
Comment 14 Iñigo Martínez 2017-09-04 19:12:37 UTC
Sure! You can merge it.

Regarding the directory order change, the answer is in the same wiki, just in the 'Addendum: Translate other XML formats':

> In your toplevel Makefile.am, make sure the po/ directory is processed before the directory that contains the MIME type file.

However, this was necessary when itstool was used to also translate 'developer_name' tag, which is not needed anymore :)
Comment 15 Iñigo Martínez 2017-09-04 19:22:13 UTC
Just as a side comment, I already made the changes suggested by Pitor in one of the prior patches[0], though it lacked the proper directory order. 

[0] https://bugzilla.gnome.org/attachment.cgi?id=359025
Comment 16 Iñigo Martínez 2017-09-04 19:24:31 UTC
(In reply to Iñigo Martínez from comment #15)
> Just as a side comment, I already made the changes suggested by *Piotr in one
> of the prior patches[0], though it lacked the proper directory order. 
> 
> [0] https://bugzilla.gnome.org/attachment.cgi?id=359025

Sorry, I mispelled the name. It's a pity that is not possible to edit those mistakes :|
Comment 17 Piotr Drąg 2017-09-04 19:31:16 UTC
(In reply to Iñigo Martínez from comment #16)
> Sorry, I mispelled the name. It's a pity that is not possible to edit those
> mistakes :|

Don’t worry about it! :)
Comment 18 Debarshi Ray 2017-09-05 06:47:40 UTC
(In reply to Iñigo Martínez from comment #14)
> Sure! You can merge it.
> 
> Regarding the directory order change, the answer is in the same wiki, just
> in the 'Addendum: Translate other XML formats':
> 
> > In your toplevel Makefile.am, make sure the po/ directory is processed before the directory that contains the MIME type file.
> 
> However, this was necessary when itstool was used to also translate
> 'developer_name' tag, which is not needed anymore :)

Ah, I see.