#6956 (Lino should refuse Choice names that are not .isidentifier())
by Luc Saffre · in Developers
Kurzbeschreibung
Until now, Choice.__init__() didn't verify whether the specified names (the third argument of the constructor) are valid Python identifiers. Now it does. Because the only purpose of these names is to make certain choices available as attributes of the choice list. But e.g. dashes aren't allowed in identifiers.
In products.DeliveryUnits we had names "m²" and "m³", which now are "m2" and "m3".
In react.PrimeReactThemes we have e.g. "bootstrap4-light-purple", which was also used as the names keyword of Choice.__init__(). We simply no longer store these strings as names. We don't need them as names.
Side effect: In the primereact_theme_name plugin setting, we want to select individual PrimeReactThemes choices, e.g. "bootstrap4-light-purple". And this setting is used only as the default value for PrimeReactThemes fields. The default value for choicelist fields can be specified as a string, and this string until now as understood as the *name* of the default choice. Not as its *value*. CallableChoice. __call__() was defined "return self.choicelist.get_by_name(self.name)". Now it's defined "return self.choicelist.get_by_name(self.name, None) or self.choicelist.get_by_value(self.name)", i.e. it works with names *or* with values. That new behaviour was needed for PrimeReactThemes where the default value is a value, not a name.
Kinder
Kommentare
- 2 Monate, 1 Woche ago von Luc Saffre about #6956 (Lino should refuse Choice names that are not .isidentifier()) : @8lurry I think that your change "give already resolved default value to SiteConfig.primereact_theme" was useless because ChoiceListField automatically resolves a default value specified as string. And I had a subtle other change there, because Lino (until then) looked up only for names (not for values) wh...
- 2 Monate, 1 Woche ago von Luc Saffre Antwort auf Luc Saffre about #6956 (Lino should refuse Choice names that are not .isidentifier()) : This was simply because I didn't release react to PyPI after fixing yesterday's issue.
- 2 Monate, 1 Woche ago von Luc Saffre about #6956 (Lino should refuse Choice names that are not .isidentifier()) (1 Antwort) : We also have it currently in the patrols CI on GitHub: Exception: bootstrap4-light-blue is not a valid identifier.
- 2 Monate, 1 Woche ago von Luc Saffre Antwort auf Sharif Mehedi about #6956 (Lino should refuse Choice names that are not .isidentifier()) : No problem! Thanks for the explanation!
- 2 Monate, 1 Woche ago von Sharif Mehedi Antwort auf Luc Saffre about #6956 (Lino should refuse Choice names that are not .isidentifier()) (1 Antwort) : Sorry about the trouble. I restored it quickly, because in @JMK machine, pm prep was failing on his machine, saying no attribute rhea on PrimeReactThemes, it's coming from the default value given to SiteConfig.primereact_theme field. I will change the default to PrimeReactThemes.get_by_value(settings_de...
- 2 Monate, 1 Woche ago von Luc Saffre Antwort auf Luc Saffre about #6956 (Lino should refuse Choice names that are not .isidentifier()) : The names argument to Choice.__init__() is used to additionally make the choices accessible using class attributes on the choicelist.
- 2 Monate, 1 Woche ago von Luc Saffre about #6956 (Lino should refuse Choice names that are not .isidentifier()) (2 Antworten) : @8lurry did you restore the line:
- super().__init__(value, value.replace('-', " ").capitalize()) + super().__init__(value, value.replace('-', " ").capitalize(), value)
Maybe this change got lost accidentally. AFAICS we should *not* use the third positional argument of __init__(). We never a... - 2 Monate, 1 Woche ago von Luc Saffre about #6956 (Lino should refuse Choice names that are not .isidentifier()) : Oops, I'm having "Exception: bootstrap4-light-blue is not a valid identifier" during run snapshot/restore.py on stewards.
- 2 Monate, 2 Wochen ago von Luc Saffre about #6956 (Lino should refuse Choice names that are not .isidentifier()) : Created new Ticket #6956 (Lino should refuse Choice names that are not .isidentifier()).