search

collective / i18ndude

i18ndude performs various tasks related to ZPT's, Python Scripts and i18n.
https://pypi.org/project/i18ndude
4 stars 9 forks source link

Add --no-line-numbers option, default false (no behavior change). #98

Closed mauritsvanrees closed 2 years ago

mauritsvanrees commented 2 years ago

Fixes https://github.com/collective/i18ndude/issues/77

Note for reviewers: a few related test classes had some lines in setUp and then one test_read method. I have merged these methods and then duplicated them into test_read_no_line_numbers and test_read_with_line_numbers. Very little of the setUp was useful for sharing.

mauritsvanrees commented 2 years ago

Perhaps we can improve the parameter name. "No line numbers false" is a bit hard to understand because of the double negation. Perhaps it could be better "Remove line numbers" .

Fair point. I had a doubt about the naming as well. I have refactored it. On the command line you now have two options: --no-line-numbers and --line-numbers. In the Python code I translate this to one parameter include_line_numbers, which by default is True. If you somehow specify both options on the command line, the last one wins. Is this better?

erral commented 2 years ago

Yes, that's right, way better.

mauritsvanrees commented 2 years ago

I have released i18ndude 5.5.0 and have updated coredev 5.2 and 6.0 to use it. I will leave it up to you to change the coredev scripts to use the new option.

erral commented 2 years ago

For the moment I will leave the po files with line numbers. I think that it is helpful to easily check where are the msgids.

Anyway, as said, the option is there.

Hau idatzi du Maurits van Rees @.***) erabiltzaileak (2022 ira. 19, al. (11:30)):

I have released i18ndude 5.5.0 and have updated coredev 5.2 and 6.0 to use it. I will leave it up to you to change the coredev scripts to use the new option.

— Reply to this email directly, view it on GitHub https://github.com/collective/i18ndude/pull/98#issuecomment-1250786039, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGHRVKPLVH245E4UP5LGXTV7AXB3ANCNFSM6AAAAAAQGGTPQE . You are receiving this because your review was requested.Message ID: @.***>

-- Mikel Larreategi @.***

CodeSyntax Azitain industrialdea, 3-K - Eibar Tfnoa.: +34 943821780

mauritsvanrees commented 2 years ago

For me, the main reason to add this option, is to avoid getting merge conflicts when one person works on translations for a language and someone else runs i18ndude and line numbers have changed. But I will leave it up to you.