search

plone / plone.i18n

Text normalization logic and language, country, cctld data.
8 stars 11 forks source link

Broken logic? #48

Closed gforcada closed 1 year ago

gforcada commented 1 year ago

See:

https://github.com/plone/plone.i18n/blob/1f9e36fffd6978e511af1164d2a7240861b93738/plone/i18n/negotiate/ptsnegotiator.py#L132-L134

This l does not raise an undeclared variable because on line 105 it gets defined.

Though, there it is surrounded by an if clause, which means that it is not 100% guaranteed that the code does run... maybe is the cause for #40 ?

In any case, I would expect that on line 134 one meant to write ll as that's the variable being used on the loop 🤔

Looking at the commit that introduced this code here, mentions that it was copied from Products.PlacelessTranslationService, and there we can see that the variable is the one used within the loop 🍀

How come, within 5 years that has not been noticed? 😅

mauritsvanrees commented 1 year ago

Issue #40 seems a Python 3 compatibility problem that was already fixed with PR #43.

But indeed, the single l should be a double ll. I guess no one notices, because all exceptions are caught a few lines further.

jensens commented 1 year ago

neither single nor double l, if at it, please give it a proper name with semantics.

mauritsvanrees commented 1 year ago

Fixed by @gforcada as part of PR #50.