Refactor helpers.py to Move Hard-Coded List to globals.py #5555
Conversation
…sible strings. new var name: _false_attributes
for more information, see https://pre-commit.ci
src/flask/globals.py
Outdated
| @@ -49,3 +49,5 @@ | |||
| session: SessionMixin = LocalProxy( # type: ignore[assignment] | |||
| _cv_request, "session", unbound_message=_no_req_msg | |||
| ) | |||
|
|
|||
| _false_attributes: t.List[str] = ["0", "false", "no", "null", "none"] | |||
There was a problem hiding this comment.
null and none do not make sense here, it's not something people would pass for a boolean option.
Also, this does not belong in this file. helpers.py is already he most suitable path.
But considering that it's already in two very simple utility functions, I'm not sure if it even makes much sense to extract this into a separate list (which, by the way, should be a set and not a list)
There was a problem hiding this comment.
it makes better practice. its not hard coded and better to maintaine.
in addition, i aggree that none and null shouldnt be passed as boolean but you can use None as False in python (in a condition. " if None" is will evaluate to False.
| @@ -49,3 +49,5 @@ | |||
| session: SessionMixin = LocalProxy( # type: ignore[assignment] | |||
| _cv_request, "session", unbound_message=_no_req_msg | |||
| ) | |||
|
|
|||
| _false_attributes: list[str] = ["0", "false", "no", "null", "none"] | |||
There was a problem hiding this comment.
This is a unmarked behavior change, this list is more exhaustive,
This might create trouble for slightly sarcastic deployments
|
I don't think this change is needed. In general, we do not accept style or refactoring PRs. |
This pull request refactors the
helpers.pyfile by moving a hard-coded list of false attributes to theglobals.pyfile. The list, previously hard-coded in multiple locations, is now centralized inglobals.pyas a new global constant named_false_attributes.Changes Made:
Benefits:
Testing: