GNOME Bugzilla – Bug 783316
Migrated from Intltool to Gettext
Last modified: 2018-05-24 11:25:04 UTC
Created attachment 352985 [details] [review] Migrated from Intltool to Gettext Intltool has been migrated to a modern Gettext version.
Review of attachment 352985 [details] [review]: Looks fine overall, I'd need to test it to double-check though.
Review of attachment 352985 [details] [review]: ::: data/appdata/Makefile.am @@ +9,1 @@ +EXTRA_DIST = $(appsdata_in_files) Should be appdata_in_files.
Created attachment 352994 [details] [review] Migrated from Intltool to Gettext Fixed typo with appdata_in_files in EXTRA_DIST. Thank you Alexandre Franke :)
Review of attachment 352985 [details] [review]: ::: po/Makevars @@ +19,3 @@ +# the public domain; in this case the translators are expected to disclaim +# their copyright. +COPYRIGHT_HOLDER = Free Software Foundation, Inc. This isn’t right. The FSF don’t hold Totem’s copyright. This should be something like “The authors of Totem”.
Review of attachment 352994 [details] [review]: ::: po/Makevars @@ +19,3 @@ +# the public domain; in this case the translators are expected to disclaim +# their copyright. +COPYRIGHT_HOLDER = Free Software Foundation, Inc. Still a problem. Looks like my review raced with your patch update.
Review of attachment 352985 [details] [review]: ::: data/Makefile.am @@ -46,3 @@ desktopdir = $(datadir)/applications desktop_DATA = $(desktop_in_files:.desktop.in=.desktop) -@INTLTOOL_DESKTOP_RULE@ You forgot to also remove the intltool-* files from the dist here. ::: src/plugins/Makefile.plugins @@ +1,2 @@ %.plugin: %.plugin.in + $(AM_V_GEN) $(MSGFMT) -L Desktop --desktop --template $< -d $(top_srcdir)/po -o $@ This doesn't work, as gettext doesn't think that the strings are translatable, so they don't appear in the pot file template, and aren't translated. ::: src/plugins/apple-trailers/apple-trailers.plugin.in @@ +3,3 @@ IAge=1 Builtin=true +Name=Apple Trailers This doesn't get translated in any of the plugin.in files. @@ +4,3 @@ Builtin=true +Name=Apple Trailers +Description=Sets the user agent for the Apple Trailers site Neither does the Description.
(In reply to Philip Withnall from comment #5) > Review of attachment 352994 [details] [review] [review]: > > ::: po/Makevars > @@ +19,3 @@ > +# the public domain; in this case the translators are expected to disclaim > +# their copyright. > +COPYRIGHT_HOLDER = Free Software Foundation, Inc. > > Still a problem. Looks like my review raced with your patch update. Almost certainly you will be right. I wasn't sure who was the proper copyright holder. I have been looking for this information and although I have some names, I haven't been able to figure out what I should write here. I looked at other projects which had "Free Software Foundation" and write it down, though I actually was very doubtful because I had no proof. In order to fix it properly, what should I write here? "The authors of Totem"? "The Totem authors"? Any other suggestion? Thanks Philip :)
A good test would be to run the application in French, it's 100% translated. If you see anything in English, it's a bug in the patch.
Created attachment 353001 [details] [review] Migrated from Intltool to Gettext Given the issues that have arised I have double checked everything to the best of my knowledge. I fixed the COPYRIGHT_HOLDER by replacing it for "Totem authors". I removed intltool-* tools from the dist. Regarding the issue of fields not translated, althought "desktop" is the most similar operation mode for plugin files, it supports only a limited set of keywords[0]. I included Name and Description as keywords to be translated and I checked the resultant files one by one. [0] http://git.savannah.gnu.org/cgit/gettext.git/tree/gettext-tools/src/write-desktop.c#n128
Review of attachment 353001 [details] [review]: ::: po/Makevars @@ +9,3 @@ + +# These options get passed to xgettext. +XGETTEXT_OPTIONS = --from-code=UTF-8 --keyword=_ --keyword=N_ --keyword=C_:1c,2 --keyword=NC_:1c,2 --keyword=g_dngettext:2,3 --add-comments If you run "make totem.pot" in the po/ directory, you'll see that the Name and Description from the plugin.in files are not getting extracted for translation, and only work because there are already translations for those in the .po files. I think you need to make it extract the "Name" and "Description" fields here. The other option is to rename all those files from foo.plugin.in to foo.plugin.desktop.in so that gettext does the right thing. That's what gedit does. That's probably the right option if you can't figure out how to do it by modifying the options above.
Usually I don't work on traslations, so I wasn't aware of this problem. I'm sorry about this. I have been examining in depth how gettext works, and more specifically the two involved tools, xgettext and msgfmt, and I have a better idea of what was happening. The first problem is related to the '.plugin' extension, which is not an extension supported by any language on gettext. This could be solved by renaming the file and adding the '.desktop' extension, but there is a second problem. The second problem is related to the Desktop Entry file default keywords[0]: Name, GenericName, Comment, Icon, Keywords. The 'Description' keyword is not included by default which produced the issue you warned me about[1]. These two problems could be solved by the following approach: 1. Rename files adding the '.desktop' extension. 2. Add 'Name' and 'Description' in the plugin description creationg which uses msgfmt. 3. Also add 'Name' and 'Description' in the XGETTEXT_OPTIONS definition at po/Makevars. I though that this was a dirty workaround, and I decided to implement a proper plugin file support for gettext, so I have already made and sent those changes[2]. I think that this could help those applications to face the same problem: gedit[3], gnome-todo[4], etc. Probably it will be a while before those patches are reviewed and a new gettext version is released. The meson port[5] was blocked due to this issue, do you think I should remove any gettext support on it ? [0] https://www.gnu.org/software/gettext/manual/html_node/xgettext-Invocation.html [1] https://bugzilla.gnome.org/show_bug.cgi?id=783316#c6 [2] http://lists.gnu.org/archive/html/bug-gettext/2017-06/msg00002.html [3] https://git.gnome.org/browse/gedit/tree/plugins [4] https://git.gnome.org/browse/gnome-todo/tree/plugins [5] https://bugzilla.gnome.org/show_bug.cgi?id=783205
(In reply to Iñigo Martínez from comment #11) > Usually I don't work on traslations, so I wasn't aware of this problem. I'm > sorry about this. > > I have been examining in depth how gettext works, and more specifically the > two involved tools, xgettext and msgfmt, and I have a better idea of what > was happening. > > The first problem is related to the '.plugin' extension, which is not an > extension supported by any language on gettext. This could be solved by > renaming the file and adding the '.desktop' extension, but there is a second > problem. The second problem is related to the Desktop Entry file default > keywords[0]: Name, GenericName, Comment, Icon, Keywords. The 'Description' > keyword is not included by default which produced the issue you warned me > about[1]. > > These two problems could be solved by the following approach: > > 1. Rename files adding the '.desktop' extension. > 2. Add 'Name' and 'Description' in the plugin description creationg which > uses msgfmt. > 3. Also add 'Name' and 'Description' in the XGETTEXT_OPTIONS definition at > po/Makevars. > > I though that this was a dirty workaround, and I decided to implement a > proper plugin file support for gettext, so I have already made and sent > those changes[2]. I think that this could help those applications to face > the same problem: gedit[3], gnome-todo[4], etc. That seems like a good course of action. Do you have a rough ETA for this? Do you think that we could have a gettext release a little while before 3.26 is released some time in September? But I'd be fine with a hack in the meanwhile, if it helps speed this along. > Probably it will be a while before those patches are reviewed and a new > gettext version is released. The meson port[5] was blocked due to this > issue, do you think I should remove any gettext support on it ? Is it not possible to pass those command-line options with meson? It's likely that we'd get a newer version of meson before a fixed gettext, so I'd go with keeping it, and applying the same hack as with autotools. Or you can break autotools altogether if the meson build is complete enough, and we'll remove the autotools support...
(In reply to Bastien Nocera from comment #12) > (In reply to Iñigo Martínez from comment #11) > > Usually I don't work on traslations, so I wasn't aware of this problem. I'm > > sorry about this. > > > > I have been examining in depth how gettext works, and more specifically the > > two involved tools, xgettext and msgfmt, and I have a better idea of what > > was happening. > > > > The first problem is related to the '.plugin' extension, which is not an > > extension supported by any language on gettext. This could be solved by > > renaming the file and adding the '.desktop' extension, but there is a second > > problem. The second problem is related to the Desktop Entry file default > > keywords[0]: Name, GenericName, Comment, Icon, Keywords. The 'Description' > > keyword is not included by default which produced the issue you warned me > > about[1]. > > > > These two problems could be solved by the following approach: > > > > 1. Rename files adding the '.desktop' extension. > > 2. Add 'Name' and 'Description' in the plugin description creationg which > > uses msgfmt. > > 3. Also add 'Name' and 'Description' in the XGETTEXT_OPTIONS definition at > > po/Makevars. > > > > I though that this was a dirty workaround, and I decided to implement a > > proper plugin file support for gettext, so I have already made and sent > > those changes[2]. I think that this could help those applications to face > > the same problem: gedit[3], gnome-todo[4], etc. > > That seems like a good course of action. Do you have a rough ETA for this? > Do you think that we could have a gettext release a little while before 3.26 > is released some time in September? > > But I'd be fine with a hack in the meanwhile, if it helps speed this along. Unfortunately, I don't have any ETA for merging the patch (if it finally happens) nor for the following gettext release, though Daiki Ueno very kindly has pointed me a possible different approach that I have to try[0]. > > Probably it will be a while before those patches are reviewed and a new > > gettext version is released. The meson port[5] was blocked due to this > > issue, do you think I should remove any gettext support on it ? > > Is it not possible to pass those command-line options with meson? It's > likely that we'd get a newer version of meson before a fixed gettext, so I'd > go with keeping it, and applying the same hack as with autotools. > > Or you can break autotools altogether if the meson build is complete enough, > and we'll remove the autotools support... Surely, it is. The first meson patch included those commands as it was a simple straight autotools conversion to meson. I've included back those commands again (along with all the improvements from the review), removing meson dependency on this patch. I have also made a new patch which adds gettext support back[1] in case plugin support is merged in gettext. [0] http://lists.gnu.org/archive/html/bug-gettext/2017-06/msg00005.html [1] https://bugzilla.gnome.org/show_bug.cgi?id=783205
Could we move on with the .plugin.desktop hack? It works fine in e.g. eog-plugins (https://git.gnome.org/browse/eog-plugins/commit/?id=d50feee91fa27732e1899c6cb6edb8653de8c441), and gettext is unlikely to get fixed any time soon. Totem is in this weird half-gettext (appdata and the main .desktop file) and half-intltool (the rest) state, which can only result in lost translations.
You are right, I'm sorry for this situation. I wanted to: 1) Revisit totem meson port. Although it works, meson has been improved and also my meson skills, so it needs to be updated/cleaned up. 2) Update my gettext patch so it follows the suggested approach. However, as you say, this isn't something that will be fixed soon. Even if `gettext` acquires a fix for this, it would be sometime until a stable version is released. I'll try to apply the `.plugin.desktop` hack as soon as possible. Lately I don't have as much free time as I'd like. I would also like to point out that the hack it's not as clean as it should be when trying to support all the features supported by meson. It has also been applied to `gnome-todo`[0] and I'm not very happy with it. [0] https://gitlab.gnome.org/GNOME/gnome-todo/merge_requests/17
Piotr, please, take a look at: https://bugzilla.gnome.org/show_bug.cgi?id=793627#c13 I have appended the patch there because it's part of that set of patches.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/totem/issues/218.