Remove line numbers from manageiq.pot and manageiq.po #22217

Fryguy · 2022-11-07T20:58:44Z

Resolves #21943

@jrafanie Please review.

Example of one of the more thorough changes:

-#: ../app/controllers/application_controller.rb:1026
-#: ../app/controllers/application_controller/ci_processing.rb:737
-#: ../app/controllers/mixins/actions/host_actions/misc.rb:55
-#: ../app/controllers/mixins/actions/host_actions/misc.rb:67
-#: ../app/controllers/mixins/actions/host_actions/misc.rb:73
-#: ../app/controllers/mixins/actions/host_actions/misc.rb:105
-#: ../app/controllers/mixins/actions/host_actions/misc.rb:116
-#: ../app/controllers/mixins/actions/host_actions/misc.rb:127
-#: ../app/controllers/mixins/actions/host_actions/misc.rb:138
+#: ../app/controllers/application_controller.rb
+#: ../app/controllers/application_controller/ci_processing.rb
+#: ../app/controllers/mixins/actions/host_actions/misc.rb
 msgid "\"%{record}\": %{task} successfully initiated"
 msgstr ""

Fryguy · 2022-11-07T21:06:34Z

This may not be the right place, but I think it's good for a first pass and we can optimize the location later possibly.

One oddity I noticed is that sometimes there are multiple references on the same line and I don't understand why (see example below). I think it's possibly an option to gettext for consolidating reference lines, but not sure where. For now, I only stripped the end, which, again, I think is fine for a first pass.

-#: ../app/controllers/vm_common.rb:628 ../app/controllers/vm_common.rb:630
+#: ../app/controllers/vm_common.rb:628 ../app/controllers/vm_common.rb
 msgid " (Analysis History)"
 msgstr ""

jrafanie · 2022-11-07T21:34:10Z

@Frank-NB Can you review this and make sure this wouldn't be a problem for you? See the examples above where the filenames would still be in the english po/pot but the line numbers would be removed.

jrafanie · 2022-11-07T21:38:18Z

lib/tasks/locale.rake

+
+      # Fix duplicate subsequent lines where there were multiple references in the same file
+      path.write(
+        (path.readlines << nil).each_cons(2).map { |l1, l2| l1 == l2 ? nil : l1 }.compact.join


Is this looking at the msgid/msgstr lines too? Should we only look at consecutive duplicate # comment lines or more specifically ones with paths?

Oh yes good point. This looks at any duplicated subsequent lines. I'd be surprised if any non-comment lines were identical, but I should probably be more tactical here.

@jrafanie Updated.

Also I took your word "consecutive" - I knew there was a better word 😆

haha, naming is hard 🤣

Resolves ManageIQ#21943

miq-bot · 2022-11-07T22:01:49Z

Some comments on commits Fryguy/manageiq@a689283~...a948118

lib/tasks/locale.rake

⚠️ - 175 - Detected puts. Remove all debugging statements.

miq-bot · 2022-11-07T22:03:30Z

Checked commits Fryguy/manageiq@a689283~...a948118 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🍰

jrafanie · 2022-11-07T22:31:29Z

@Frank-NB Can you review this and make sure this wouldn't be a problem for you? See the examples above where the filenames would still be in the english po/pot but the line numbers would be removed.

@Frank-NB what I'm asking is does this cause any problems with the translation tools you're using. See the attached update po/pot. If we change this, we want to make sure your tools will not have problems with the commented files+line number locations.

Frank-NB · 2022-11-08T01:39:45Z

@Frank-NB Can you review this and make sure this wouldn't be a problem for you? See the examples above where the filenames would still be in the english po/pot but the line numbers would be removed.

@Frank-NB what I'm asking is does this cause any problems with the translation tools you're using. See the attached update po/pot. If we change this, we want to make sure your tools will not have problems with the commented files+line number locations.

Hi @jrafanie , I think it is ok for translation tool, we use msgid as key, and msgstr as value.

jrafanie · 2022-11-11T15:29:18Z

lib/tasks/locale.rake

@@ -207,6 +221,9 @@ namespace :locale do
    system('rmsgmerge', '--sort-by-msgid', '--no-fuzzy-matching', '-o', Rails.root.join('locale', 'en', 'manageiq-all.po').to_s, Rails.root.join('locale', 'en', 'manageiq.po').to_s, Rails.root.join('locale', 'manageiq.pot').to_s)
    system('mv', '-v', Rails.root.join('locale', 'en', 'manageiq-all.po').to_s, Rails.root.join('locale', 'en', 'manageiq.po').to_s)
    system('rm', '-rf', tmp_dir)
+
+    remove_line_numbers(Rails.root.join('locale/manageiq.pot'))
+    remove_line_numbers(Rails.root.join('locale/en/manageiq.po'))


I think this should also remove the lines numbers in the translations:

locale/pt_BR/manageiq.po locale/ja/manageiq.po locale/it/manageiq.po locale/zh_TW/manageiq.po locale/zh_CN/manageiq.po locale/de/manageiq.po locale/ko/manageiq.po locale/fr/manageiq.po locale/es/manageiq.po

In addition to what it's already doing:

locale/en/manageiq.po locale/manageiq.pot

Then the translators add or remove from the translations and these translations come back without line numbers.

Is this doable? Is this a problem?

I do want to get this in as I think removing line numbers will help see the actual changes to all of these files.

@Frank-NB Can you verify if it's ok for us to remove line numbers in the translations too?

I can definitely do that but it should happen automatically. That is, I'd expect the translators to take the .pot files and generate the .po from it, so those line numbers are driven by the .pot. (Though we can do a first pass for diff purposes manually of course, but it doesn't have to be this PR)

Exactly, I'd like @Frank-NB's opinion as I'm unsure how they create/update the translation files.

Hi @jrafanie , Yes, we take the .pot files and generate tho .po from it, so I think if you update en/manageiq.po and manageiq.pot, translations will come back without line numbers. but I think we need to test it after this PR merged, hope it can work as design.

Considering they are only comments, this should not affect anything from working. We had considered removing the comments entirely, but I think having at least the files is useful diagnostically.

Fryguy · 2022-11-11T17:54:31Z

cc @jtux270

jrafanie · 2022-11-14T13:46:52Z

Ok, I'm merging this @Frank-NB... the next round of po/pot will be missing line numbers so please let us know if you run into problems with it.

Fryguy · 2022-11-18T21:59:13Z

Backported to oparin in commit f2f3b85.

commit f2f3b854f8bbc889ba612a957082c6a0eca53020
Author: Joe Rafaniello <jrafanie@users.noreply.github.com>
Date:   Mon Nov 14 08:46:57 2022 -0500

    Merge pull request #22217 from Fryguy/locale_remove_line_numbers
    
    Resolves https://github.com/ManageIQ/manageiq/issues/21943
    
    (cherry picked from commit abe57a0175bb6d2818a6d87d3e130259a04bb3f3)

Resolves #21943 (cherry picked from commit abe57a0)

Fryguy requested a review from jrafanie as a code owner November 7, 2022 20:58

Fryguy assigned jrafanie Nov 7, 2022

Fryguy added the internationalization label Nov 7, 2022

Fryguy force-pushed the locale_remove_line_numbers branch from 9fcb7f7 to c89715b Compare November 7, 2022 21:02

jrafanie reviewed Nov 7, 2022

View reviewed changes

Fryguy force-pushed the locale_remove_line_numbers branch 2 times, most recently from 1539633 to b71ea52 Compare November 7, 2022 21:55

Fryguy added 2 commits November 7, 2022 16:56

Remove line numbers from manageiq.pot and manageiq.po

a689283

Resolves ManageIQ#21943

Update manageiq.pot and manageiq.po with removed line numbers

a948118

Fryguy force-pushed the locale_remove_line_numbers branch from b71ea52 to a948118 Compare November 7, 2022 21:56

jrafanie reviewed Nov 11, 2022

View reviewed changes

jrafanie mentioned this pull request Nov 11, 2022

Add github action PR for locale:update_all #22227

Merged

jrafanie merged commit abe57a0 into ManageIQ:master Nov 14, 2022

Fryguy deleted the locale_remove_line_numbers branch November 14, 2022 14:07

Fryguy mentioned this pull request Nov 14, 2022

Remove line numbers from all translation files #22231

Merged

Fryguy restored the locale_remove_line_numbers branch November 14, 2022 20:21

Fryguy added the oparin/yes label Nov 15, 2022

Fryguy deleted the locale_remove_line_numbers branch November 15, 2022 12:51

Fryguy pushed a commit that referenced this pull request Nov 18, 2022

Merge pull request #22217 from Fryguy/locale_remove_line_numbers

f2f3b85

Resolves #21943 (cherry picked from commit abe57a0)

Fryguy added oparin/backported and removed oparin/yes labels Nov 18, 2022

Fryguy mentioned this pull request Nov 8, 2023

Update translations from 10/26 English catalog #22781

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove line numbers from manageiq.pot and manageiq.po #22217

Remove line numbers from manageiq.pot and manageiq.po #22217

Fryguy commented Nov 7, 2022 •

edited

Fryguy commented Nov 7, 2022

jrafanie commented Nov 7, 2022

jrafanie Nov 7, 2022

Fryguy Nov 7, 2022 •

edited

Fryguy Nov 7, 2022

Fryguy Nov 7, 2022

jrafanie Nov 7, 2022

miq-bot commented Nov 7, 2022

miq-bot commented Nov 7, 2022

jrafanie commented Nov 7, 2022

Frank-NB commented Nov 8, 2022

jrafanie Nov 11, 2022

jrafanie Nov 11, 2022

jrafanie Nov 11, 2022

Fryguy Nov 11, 2022

jrafanie Nov 11, 2022

Frank-NB Nov 11, 2022 •

edited

Fryguy Nov 11, 2022

Fryguy commented Nov 11, 2022

jrafanie commented Nov 14, 2022

Fryguy commented Nov 18, 2022

Remove line numbers from manageiq.pot and manageiq.po #22217

Remove line numbers from manageiq.pot and manageiq.po #22217

Conversation

Fryguy commented Nov 7, 2022 • edited

Fryguy commented Nov 7, 2022

jrafanie commented Nov 7, 2022

Choose a reason for hiding this comment

Fryguy Nov 7, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Nov 7, 2022

miq-bot commented Nov 7, 2022

jrafanie commented Nov 7, 2022

Frank-NB commented Nov 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Frank-NB Nov 11, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fryguy commented Nov 11, 2022

jrafanie commented Nov 14, 2022

Fryguy commented Nov 18, 2022

Fryguy commented Nov 7, 2022 •

edited

Fryguy Nov 7, 2022 •

edited

Frank-NB Nov 11, 2022 •

edited