Re: [PATCH] gettext: handle unsupported languages properly

[Petr Lautrbach <plautrba@redhat.com>]

Petr Lautrbach <plautrba@redhat.com> writes:

> Daniel Burgener <dburgener@linux.microsoft.com> writes:
>
>> On 6/24/2022 1:27 PM, Vit Mojzis wrote:
>>> 
>>> 
>>> On 6/24/22 18:37, Daniel Burgener wrote:
>>>> On 6/24/2022 10:24 AM, Vit Mojzis wrote:
>>>>> With "fallback=True" gettext.translation behaves the same as
>>>>> gettext.install and uses NullTranslations in case the
>>>>> translation file for given language was not found (as opposed to
>>>>> throwing an exception).
>>>>>
>>>>> Fixes:
>>>>>    # LANG is set to any "unsupported" language, e.g. en_US.UTF-8
>>>>>    $ chcat --help
>>>>>    Traceback (most recent call last):
>>>>>    File "/usr/bin/chcat", line 39, in <module>
>>>>>      t = gettext.translation(PROGNAME,
>>>>>    File "/usr/lib64/python3.9/gettext.py", line 592, in translation
>>>>>      raise FileNotFoundError(ENOENT,
>>>>>    FileNotFoundError: [Errno 2] No translation file found for domain: 
>>>>> 'selinux-python'
>>>>>
>>>>> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
>>>>> ---
[...]
>>>>
>>>> Isn't the point of the overall change that gettext.translation() 
>>>> doesn't throw an exception anymore?  So we don't need to handle 
>>>> OSError/IOError here once fallback=True.  I see that this standardizes 
>>>> with the other call sites, but I wonder if standardizing on the more 
>>>> specific exceptions (or just leaving as is) wouldn't be better?
>>> 
>>> Yes, we do not need to handle OSError/IOError exceptions any more.
>>> I agree that standardizing on the more specific exception handling would 
>>> be more proper. However, translation handling is probably the least 
>>> important feature of this tool (most people probably use it in English 
>>> and those who don't wish they did) -- we don't want people to be unable 
>>> to use the tool at all because of some translation issue and "catch all" 
>>> exception handling is the easiest way to avoid that.
>>> As this bug showed, not settling on a single approach is probably the 
>>> worst alternative (I tested the previous patch with multiple tools, but 
>>> apparently missed the one that was different).
>>> 
>>> That being said, either approach is fine by me. Please let me know if I 
>>> should change the patch.
>>> Vit
>>
>> Well, I'll leave questions of what you *should* do up to the 
>> maintainers, but I think as far as I'm concerned your point that we 
>> really don't want to fail hard on translations makes a lot of sense (and 
>> I definitely agree that inconsistency is a pretty bad alternative.  So 
>> I'm on board with this approach.  I was initially worried about "what if 
>> in some future version gettext adds an exception?", but to your point, 
>> we want the same fallback logic in that case, so the general except 
>> seems reasonable.
>>
>> Reviewed-by: Daniel Burgener <dburgener@linux.microsoft.com>
>>
>
> Thanks!
>
> Acked-by: Petr Lautrbach <plautrba@redhat.com>
>
>


Merged.