Petr Lautrbach <plautrba@xxxxxxxxxx> writes: > Daniel Burgener <dburgener@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx> >>>>> --- [...] >>>> >>>> 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@xxxxxxxxxxxxxxxxxxx> >> > > Thanks! > > Acked-by: Petr Lautrbach <plautrba@xxxxxxxxxx> > > Merged.
