Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 928431 - app-i18n/translate-shell: what do you think about my patch that in voicing disable stupid prefix "Translations of"?
Summary: app-i18n/translate-shell: what do you think about my patch that in voicing di...
Status: UNCONFIRMED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Ferenc Erki
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-04-02 14:57 UTC by Vitaly Zdanevich
Modified: 2024-05-12 14:01 UTC (History)
3 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
Patch, should be applied when some new USE flag is applied (trans.patch,55.19 KB, patch)
2024-04-02 14:57 UTC, Vitaly Zdanevich
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vitaly Zdanevich 2024-04-02 14:57:26 UTC
Created attachment 889258 [details, diff]
Patch, should be applied when some new USE flag is applied

See issue about that on the upstream https://github.com/soimort/translate-shell/issues/408
Comment 1 Vitaly Zdanevich 2024-04-02 14:58:17 UTC
It works for me, I use it on my machine, want to help other users too.
Comment 2 Ferenc Erki 2024-05-12 10:03:27 UTC
Thank you for highlighting the idea and sharing an example patch!

Overall I would prefer addressing this upstream instead of making Gentoo's downstream version special if an opt-in is used.

When upstream decides to merge a patch for this, we may be able to backport/import that to Gentoo before an official upstream release is made, or perhaps we can make a separate snapshot ebuild early.

Until then, I believe this is a good candidate to apply as a user patch instead by saving it under /etc/portage/patches (see https://wiki.gentoo.org/wiki/.etc/portage/patches for more details).

Second, about the patch itself:

1. The main logic seems to be to skip fetching any of the Locale[*]["translations-of"] entries. These seem to be not retrieved anymore, while also commented out. In case these could stay, that would considerably reduce the size of the patch, while also minimizing the deviance compared to upstream. Is it necessary to comment all 103 variations of the Locale[*]["translations-of"] entries?

2. With an exception of a single added line, every change in the diff is commenting something out. If we could delete the commented lines instead, it would also considerably reduce the patch size. Is it necessary to keep the commented out lines in the patched result instead of deleting them?
Comment 3 Vitaly Zdanevich 2024-05-12 11:38:52 UTC
> Overall I would prefer addressing this upstream instead of making Gentoo's downstream version special if an opt-in is used.

Me too, but upstream is not answering to this https://github.com/soimort/translate-shell/issues/408
Comment 4 Vitaly Zdanevich 2024-05-12 11:40:52 UTC
If you agree to merge this patch, I can edit it as you want.
Comment 5 Vitaly Zdanevich 2024-05-12 11:42:25 UTC
> If we could delete the commented lines instead, it would also considerably reduce the patch size

Maybe add a comment to every commented line like "It was commented by Gentoo patch <link>"?
Comment 6 Vitaly Zdanevich 2024-05-12 11:45:55 UTC
> Is it necessary to keep the commented out lines in the patched result instead of deleting them?

Maybe deleting is better for translate-shell performance on very slow systems.
Comment 7 Ferenc Erki 2024-05-12 14:01:54 UTC
TL;DR:

The attached patch:

- does not seem to be correct to me, because it potentially affects more than just the text-to-speech functionality, and more complex than it minimally has to be
- seems too large to be included in the main tree (55kB vs 20kB)
- still may be useful as a local user patch whenever a quick override is desired

Longer version:

(In reply to Vitaly Zdanevich from comment #3)
> Me too, but upstream is not answering to this
> https://github.com/soimort/translate-shell/issues/408

Still, they are the ones who know how to address this properly in an acceptable way. It might worth proposing the patch there to ask for further feedback.

(In reply to Vitaly Zdanevich from comment #4)
> If you agree to merge this patch, I can edit it as you want.

I think the current patch posted here is useful for others to use it as a user-side quick-fix patch as mentioned above. They can find it here, and decide if it's useful for them or not.

The more I look at the source code and the proposed patch, the less comfortable I am to merge it as "the Gentoo" solution in its current form.

The proposed patch seems to disable returning translations for the "translations of" text for every translate-shell functionality which may call it. I feel that accidentally disables other functionality than solely the text-to-speech one.

(In reply to Vitaly Zdanevich from comment #5)
> > If we could delete the commented lines instead, it would also considerably reduce the patch size
> 
> Maybe add a comment to every commented line like "It was commented by Gentoo
> patch <link>"?

I would prefer leaving the translated texts alone, and just replace the logic (instead of commenting it out and adding a new one).

Then again, I am not convinced that essentially skipping the whole showTransationsOf() function is the correct approach to achieve the desired results without side effects.

(In reply to Vitaly Zdanevich from comment #6)
> > Is it necessary to keep the commented out lines in the patched result instead of deleting them?
> 
> Maybe deleting is better for translate-shell performance on very slow
> systems.

I believe it's more about the size of the patch Gentoo has to carry for every Gentoo system, even for the ones not using translate-shell. It's also harder to understand and maintain a larger patch than a small one.

As far as I can tell, the size limit for in-tree patches is currently 20kB, and pkgcheck would complain above that. The currently proposed patch is 55kB. At that size, it either needs to be hosted elsewhere, or made smaller.