Skip to content

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Jun 14, 2021

locale_compose() accepts at most 15 'variant' and 'private' subtags
each, and at most 3 'extlang' subtags. This is currently only enforced
if the subtags are given as specific key (e.g. 'variant0'); for
consistency we enforce this for generic keys (e.g. 'variant'), too.


This patch matches the documentation, but it seems to me that the documentation is wrong. I assume the limitations have been introduced to avoid "infinite" hash table lookups (although that could have been solved differently), but that is not an issue when the generic keys are used with arrays.

Maybe @smalyshev can clarify?

`locale_compose()` accepts at most 15 'variant' and 'private' subtags
each, and at most 3 'extlang' subtags.  This is currently only enforced
if the subtags are given as specific key (e.g. 'variant0'); for
consistency we enforce this for generic keys (e.g. 'variant'), too.
@cmb69 cmb69 added the Bug label Jun 14, 2021
@nikic
Copy link
Member

nikic commented Jun 15, 2021

This patch matches the documentation, but it seems to me that the documentation is wrong. I assume the limitations have been introduced to avoid "infinite" hash table lookups (although that could have been solved differently), but that is not an issue when the generic keys are used with arrays.

I agree. After reading through the code, limiting the number of keys we try to check seems to be the only reason for this limit, so extending it to the case where we don't have to probe multiple keys wouldn't make sense to me.

If we want to make a change here, it should be in removing the limitation by iterating over the HT and matching keys, rather than looking up specific keys. If we're not interested in that, I'd just mark the original report as "not a bug" and maybe clarify docs to distinguish when the limit doesn't apply.

@cmb69
Copy link
Member Author

cmb69 commented Jun 15, 2021

If we want to make a change here, it should be in removing the limitation by iterating over the HT and matching keys, rather than looking up specific keys.

ACK. However, to retain the current behavior, we would also need to sort the entries by suffix for BC reasons(e.g. extlang0 always has to be used before extlang1 regardless of their order in the HT). I think it's best to leave that as is. IMO, if we wanted to change something here, we should drop those suffixed subtags, but that would probably need deprecation …

@cmb69 cmb69 closed this Jun 15, 2021
@cmb69 cmb69 deleted the cmb/54176 branch June 15, 2021 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants