Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from intltool to gettext #512

Merged
merged 6 commits into from Dec 1, 2019
Merged

Migrate from intltool to gettext #512

merged 6 commits into from Dec 1, 2019

Conversation

yetist
Copy link
Member

@yetist yetist commented Nov 10, 2019

This PR is not finished, please don't merge it.

I am having trouble and need some help. I don't know how to write a correct .its file for some xml files. These files in plugins/taglist direcoty, named *.tags.xml.in, for example:

<?xml version="1.0" encoding="UTF-8"?>
<TagList xmlns="http://pluma.sourceforge.net/some-location">
<TagGroup name="XHTML 1.0 - Tags" sort="true">

  <Tag name="Abbreviated form">
    <Begin>&lt;abbr title="</Begin>
    <End>"&gt;&lt;/abbr&gt;</End>
  </Tag>
...

I don't know how to make name properties in Tag translatable, the string is Abbreviated form.

@mate-desktop/core-team @mate-desktop/contributors
Who can help? Or do we need to modify the code to change the xml file structure?

BTW, {it,ru,uk}.po has errors, I temporarily disable them.

@sc0w
Copy link
Member

sc0w commented Nov 11, 2019

I fixed it language at transifex

@monsta ru and uk have the same problems mentioned in mate-desktop/eom#245

  • affected lines in ru:
#: ../pluma/pluma-statusbar.c:352
#, c-format
msgid "There is a tab with errors"
msgid_plural "There are %d tabs with errors"
msgstr[0] "Есть вкладка с ошибками"
msgstr[1] "%d вкладок с ошибками"
msgstr[2] "%d вкладок с ошибками"
msgstr[3] "%d вкладок с ошибками"
  • affected lines in uk:
#: ../pluma/pluma-commands-file.c:1165
#, c-format
msgid ""
"Changes made to the document in the last %ld minute will be permanently "
"lost."
msgid_plural ""
"Changes made to the document in the last %ld minutes will be permanently "
"lost."
msgstr[0] ""
"Зміни, внесені у документ за останню хвилину, будуть остаточно втрачені."
msgstr[1] ""
"Зміни, внесені у документ за останні %ld хвилини, будуть остаточно втрачені."
msgstr[2] ""
"Зміни, внесені у документ за останні %ld хвилин, будуть остаточно втрачені."
msgstr[3] ""
"Зміни, внесені у документ за останні %ld хвилин, будуть остаточно втрачені."
#: ../pluma/pluma-statusbar.c:352
#, c-format
msgid "There is a tab with errors"
msgid_plural "There are %d tabs with errors"
msgstr[0] "Вкладка з помилками"
msgstr[1] "%d вкладки з помилками"
msgstr[2] "%d вкладок з помилками"
msgstr[3] "%d вкладок з помилками"

@monsta
Copy link
Contributor

monsta commented Nov 12, 2019

Ok, try #513 for a fix.

@raveit65
Copy link
Member

raveit65 commented Nov 12, 2019

@yetist
Can you please fix the issue in your way with adding /* xgettext:no-c-format */ to .pot file and removing format-c from .mo files ?
I want avoid to remove the translated string for all other languages, which will happen with #513

@raveit65
Copy link
Member

I don't know how to make name properties in Tag translatable, the string is Abbreviated form.

Sometimes a look at gedit repo at gitlab helps. I guess gedit is ported to gettext.

@yetist
Copy link
Member Author

yetist commented Nov 13, 2019

pluma/po/it.po

Lines 762 to 768 in a6fe4f4

"Se non si salva, le modifiche apportate nell'ultimo &ld minuto saranno perse"
" per sempre."
msgstr[1] ""
"Se non si salva, le modifiche apportate negli ultimi &ld minuti saranno "
"perse per sempre."
#: ../pluma/dialogs/pluma-close-confirmation-dialog.c:454

Typing error make it.po fail. &ld should be %ld.

@yetist
Copy link
Member Author

yetist commented Nov 13, 2019

@yetist
Can you please fix the issue in your way with adding /* xgettext:no-c-format */ to .pot file and removing format-c from .mo files ?

Done with a46ab61, and this change fix errors from ru.po and uk.po. Now only one error from it.po.

@yetist
Copy link
Member Author

yetist commented Nov 13, 2019

@raveit65

Sadly, gedit droped the tag list plugin.

GNOME/gedit@c7cf365
GNOME/gedit-plugins@7f700d3

@sc0w
Copy link
Member

sc0w commented Nov 13, 2019

@yetist

Typing error make it.po fail. &ld should be %ld.

It was already fixed at transifex

EDIT: uk has two errors, one was already fixed, and the last error will be fixed with #513

@sc0w
Copy link
Member

sc0w commented Nov 13, 2019

I don't agree with removing format-c, the uk and ru translations are wrong and it doesn't fix it. (see mate-desktop/eom#245 (comment) )

@raveit65
Copy link
Member

raveit65 commented Nov 13, 2019

I don't agree with removing format-c, the uk and ru translations are wrong and it doesn't fix it. (see mate-desktop/eom#245 (comment) )

#513 will drop translations for other languages eg. de, fr and a lot of others. I posted the links already.
But for you, broken french translation mate-desktop/eom@f620f3d#diff-42793ab00dcd448b33776fc86e5eaaf4R1263
And i fixed de translation yesterday.
Again, it is not acceptable to broke a lot of other translations for only fixing 2 languages.
Please let us test the fix from here.

@raveit65
Copy link
Member

Rebase with fixed it and uk translation from master.
@sc0w did forget to pull and commit his fixes from transifex at master.

@sc0w
Copy link
Member

sc0w commented Nov 13, 2019

@raveit65

if you remove format-c, in the future we can't see translation problems, for example if source have %d and translator use $d , the errors are ignored, and it can make unexpected behavior.

Again, it is not acceptable to broke a lot of other translations for only fixing 2 languages.

It doesn't break other translations, it just add a little more work to translators, and I think they are happy doing the job. Is acceptable to have wrong translations in two languages forever?

@sc0w did forget to pull and commit his fixes from transifex at master.

I just was waiting for #513 to be merged

@mate-desktop/core-team I think we can vote, do you like to remove format-c and ignore all the gettext errors?

@yetist
Copy link
Member Author

yetist commented Nov 13, 2019

I think we should do things with the right way. If it is wrong, then we will correct it instead of ignoring it.
So, if msgstr[0] in po removes %d is wrong, then we should add it instead of removing c-format to ignore it.

@sc0w
Copy link
Member

sc0w commented Nov 13, 2019

@mate-desktop/core-team

I think the right way, to make everybody happy, is don't change ngettext code, fix it at transifex, and ignore de wrong error in transifex.

@raveit65
Copy link
Member

raveit65 commented Nov 13, 2019

Sigh,
all i know is that singular form of translation was removed from german and french and a lot of other languages in eom.
It looks like that %d was added to singular form in eom.

I only asked to test the other way of fixing the problem in PR.
But it seems this is too much for a straight guy like you.
Always asking for a vote from people is not the right way to argument and working together
But feel free to work alone on this PR and others if you know it better.
I am out.

Edit: and unsubscribed

@sc0w
Copy link
Member

sc0w commented Nov 13, 2019

I fixed ru at transifex

but I couldn't fix ukranian (uk)

2019-11-13_12-01_1

I click in Save changes, but it never saves.

I tried to contact to transifex team, but they doesn't reply by mail, and I don't know how to register to open new thread in https://community.transifex.com/c/translators-community

@monsta
Copy link
Contributor

monsta commented Nov 13, 2019

Again, it is not acceptable to broke a lot of other translations for only fixing 2 languages.

List of languages with one of the plural forms in msgstr[0] (where the formula returns 0):
be, bs, hr, lt, lv, ru, sl, sr, sr@latin, uk.

@yetist yetist changed the title [WIP] Migrate from intltool to gettext Migrate from intltool to gettext Nov 14, 2019
@yetist yetist requested a review from a team November 14, 2019 06:35
@yetist
Copy link
Member Author

yetist commented Nov 14, 2019

This PR is done, waiting for the fix of ru.po and uk.po.

@sc0w
Copy link
Member

sc0w commented Nov 14, 2019

@mate-desktop/core-team maybe we must to migrate from transifex to other alternative?

@sc0w
Copy link
Member

sc0w commented Nov 14, 2019

transifex issue was reported in 2013! here: transifex/transifex-old-core#287

@monsta
Copy link
Contributor

monsta commented Nov 14, 2019

Check out README.md there, that repo isn't maintained for years...

@sc0w
Copy link
Member

sc0w commented Nov 14, 2019

oops, yes, seems the issue never will be fixed in transifex

@monsta
Copy link
Contributor

monsta commented Nov 14, 2019

Maybe try https://www.transifex.com/contact/?

@sc0w
Copy link
Member

sc0w commented Nov 15, 2019

The job is done!

I downloaded the .po file from transifex, modify, and later I uploaded it.

2019-11-15_02-29

@monsta Please check if PR is ok for you

@sc0w sc0w requested a review from monsta November 15, 2019 01:37
@monsta
Copy link
Contributor

monsta commented Nov 15, 2019

It's ok as a quickfix, but the same can happen with other languages I've mentioned when they'll get the translation for these strings.

Copy link
Contributor

@monsta monsta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yetist: same as in mate-desktop/eom#245 (comment), make distclean doesn't remove the generated files. It worked with intltool. Now I have to remove all of them manually, otherwise nothing is built in po subdir on the next builds.

@sc0w
Copy link
Member

sc0w commented Nov 15, 2019

@monsta

It's ok as a quickfix, but the same can happen with other languages I've mentioned when they'll get the translation for these strings.

Yes, and we can again download-fix-upload, I think change code to make happy transifex which doesn't work as expected is a bad idea. In the future, if transifex don't like to fix the issue we can consider alternatives (maybe https://crowdin.com ?)

@yetist
Copy link
Member Author

yetist commented Nov 16, 2019

@sc0w
Copy link
Member

sc0w commented Nov 16, 2019

@yetist I see this warning in logs:

/usr/bin/xgettext: warning: file 'pluma/pluma-ui.xml' extension 'xml' is unknown; will try C

@monsta
Copy link
Contributor

monsta commented Nov 19, 2019

@yetist: oh, great. So the discussion went nowhere and then https://bugs.debian.org/400453 was archived in 2007...

Ok, I get it about .gmo files. I've also found that make maintainer-clean removes them. But other generated files are never cleaned.

monsta@asylum:~/mate/mate-session-manager$ LC_ALL=C git status po 
On branch master
Your branch is up-to-date with 'origin/master'.
Untracked files:
  (use "git add <file>..." to include in what will be committed)

	po/Makefile.in.in
	po/Makevars.template
	po/Rules-quot
	po/boldquot.sed
	po/en@boldquot.header
	po/en@quot.header
	po/insert-header.sin
	po/mate-session-manager.pot
	po/quot.sed
	po/remove-potcdate.sin

nothing added to commit but untracked files present (use "git add" to track)

@raveit65
Copy link
Member

@yetist
Several desktop files of modules have Icon entries. Can you please add translator hints?

+#: plugins/filebrowser/filebrowser.plugin.desktop.in:8
+msgid "system-file-manager"
+msgstr ""

+#: plugins/pythonconsole/pythonconsole.plugin.desktop.in:8
+msgid "text-x-python"
+msgstr ""

+#: plugins/quickopen/quickopen.plugin.desktop.in:8
+msgid "document-open"
+msgstr ""

+#: plugins/sort/sort.plugin.desktop.in:7
+msgid "view-sort-ascending"
+msgstr ""

+#: plugins/spell/spell.plugin.desktop.in:7
+msgid "tools-check-spelling"
+msgstr ""

+#: plugins/trailsave/trailsave.plugin.desktop.in:7
+msgid "edit-cut"
+msgstr ""

pluma-plugin.desktop.in has an Icon entry too, but i don't find the msgid in new pluma.pot resource.

@raveit65
Copy link
Member

offtopic on
Looks like there is an issue in python code (not related to PR), i get this warning when gettext creates the pluma.pot file.

plugins/snippets/snippets/Placeholder.py:609: warning: 'msgid' format string with unnamed arguments cannot be properly localized:
                                                       The translator cannot reorder the arguments.
                                                       Please consider using a format string with named arguments,
                                                       and a mapping instead of a tuple for the arguments.

offtopic off

@raveit65
Copy link
Member

Hmm, this happens when i select the tag-list plugin in sidebar.

[rave@mother ~]$ pluma 

** (pluma:22512): WARNING **: 16:04:57.491: The tag list file '/usr/share/pluma/plugins/taglist/HTML.tags.gz' is of the wrong type, was 'comment', 'Tag' expected.

** (pluma:22512): WARNING **: 16:04:57.491: The tag list file '/usr/share/pluma/plugins/taglist/HTML.tags.gz' is of the wrong type, error parsing TagGroup 'XHTML 1.0 - Tags'.

** (pluma:22512): WARNING **: 16:04:57.491: The tag list file '/usr/share/pluma/plugins/taglist/HTML.tags.gz' is of the wrong type, was 'comment', 'Tag' expected.

** (pluma:22512): WARNING **: 16:04:57.491: The tag list file '/usr/share/pluma/plugins/taglist/HTML.tags.gz' is of the wrong type, error parsing TagGroup 'HTML - Tags'.

This doesn't happen without PR.

When xml file is generated using intltool, the resulting xml file deletes the
comment line, but when using gettext, it keeps the comment line.
When using intltool, the taglist creates a big xml file containing the
local translation string, so that it has to be compressed.

It looks like this:
```
<TagGroup sort="true" name="XHTML 1.0 - Tags">
  <Tag name="Abbreviated form">
    ...
  </Tag>
  <Tag name="...">
    ...
  </Tag>
  ...
</TagGroup>
<TagGroup xml:lang="af" sort="true" name="XHTML 1.0 - Tags">
  <Tag name="Afgekorte vorm" xml:lang="af">
    ...
  </Tag>
  <Tag name="..." xml:lang="af">
    ...
  </Tag>
  ...
</TagGroup>
<TagGroup xml:lang="..." sort="true" name="XHTML 1.0 - Tags">
</TagGroup>
```

Obviously, it wastes space and download bandwidth.

When switch from intltool to gettext, it does not generate a similar
huge xml file. It only get the translate string into pot file from the
xml file, and the original xml file has not changed.

This patch let taglist-plugin read the local translation string directly
from the mo file, so that it can work as before.

TODO: we need to improve and optimize the taglist plugin code and
drop some extra code.
Copy link
Member

@raveit65 raveit65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,
taglist-plugin warnings are fixed now.
./autogen.sh && ./makepot
diff -uprN pluma-master.pot pluma.pot > diff-pot-files.diff
shows me that all translations are there, and all desktop files have translator hints.
Installation/uninstallation to /usr/local/share/locale/*/LC_MESSAGES/*.mo works fine.
All necessary files are in a tarball generated by distcheck.
I don't mind about distclean remarks because we won't use it with meson build system in future.
... ready to go.
Thanks.

@raveit65 raveit65 requested a review from a team November 25, 2019 16:51
@raveit65
Copy link
Member

raveit65 commented Dec 1, 2019

Approved by 2 peoples is enough.

@raveit65 raveit65 merged commit f939df1 into master Dec 1, 2019
@raveit65 raveit65 deleted the gettext branch December 1, 2019 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants