Honour the valid_empty flag even when Input has no validators.#5882
Honour the valid_empty flag even when Input has no validators.#5882holdenweb wants to merge 2 commits into
valid_empty flag even when Input has no validators.#5882Conversation
holdenweb
changed the title
valid_empty flag even when Input has no validators.valid_empty flag even when Input has no validators. [WIP]
|
The logic of the present If the field has an empty validator set, the assumption appears to be that It is also unclear why, under these circumstances, an empty value with The Reversing the sequence in which the initial steps of validation are applied still passes all current tests, would appear to be backwards compatible, and ensures that the |
Formerly an Input with `valid_empty=True` and no validators would validate successfully even with an empty value.
holdenweb
changed the title
valid_empty flag even when Input has no validators. [WIP]valid_empty flag even when Input has no validators.
| def compose(self) -> ComposeResult: | ||
| yield Input(valid_empty=False, value="x") | ||
|
|
||
| async def test_invalid_empty(): |
There was a problem hiding this comment.
You'll need an async with here.
| self._reactive_valid_empty = valid_empty | ||
| self._valid = True | ||
|
|
||
| self._valid = not (input and not valid_empty) |
There was a problem hiding this comment.
I find the negation requires a bit of mental parsing. Maybe self._valid = valid_empty and not input ?
There was a problem hiding this comment.
More importantly, I presume you meant value rather than input here?
There was a problem hiding this comment.
I take your point. However, I feel we really should use an equivalent expression :)
>>> for valid_empty in (True, False):
... for input in (True, False):
... print(f"{valid_empty=} {input=} {(not (input and not valid_empty), valid_empty and not input)}")
...
valid_empty=True input=True (True, False)
valid_empty=True input=False (True, True)
valid_empty=False input=True (False, False)
valid_empty=False input=False (True, False)
I'll use valid_empty or not input. de Morgan's law FTW!
There was a problem hiding this comment.
Clearly this PR still needs a bit of work ...
There was a problem hiding this comment.
Sorry you've lost me. My point is that input is just an in-built Python function and has no relation to the Input in Textual.
TomJGooding
left a comment
TomJGooding
left a comment
There was a problem hiding this comment.
Sorry if this has already been discussed over on Discord, but I don't understand this PR at all I'm afraid.
| self._reactive_valid_empty = valid_empty | ||
| self._valid = True | ||
|
|
||
| self._valid = not (input and not valid_empty) |
There was a problem hiding this comment.
More importantly, I presume you meant value rather than input here?
|
I'm sorry I wasted your time with this PR, which as you have observed is completely mangled. The actual fix required is simple, and will be the subject of a separate pull request coming shortly. |
|
No point going any further here. |
3 participants
Please review the following checklist.
Discussed this in Discord.
This patch ensures that even without validators an empty input with
valid_emptyset toFalsewill be rejected.