Assorted tweaks to Input
#2743
Merged
Assorted tweaks to Input
#2743
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also remove the parameter that wasn't being used anyway.
The branch being removed here seems to be trying to handle keyboard bindings before handling the raw keyboard event. However, in the unit tests, even when a binding is pressed, this code doesn't get called. I strongly suspect this is code that predates changes that were made some time ago in respect to the order in which bindings were processed and their relationship to keystrokes. After removing this all tests are passing just fine and hand-testing `Input` (especially in the demo, for example, both non-password and password incarnations) shows no problems either. All evidence suggests that Textualize#2737 was incapable of hitting that branch of code because it just could not be hit these days.
willmcgugan
approved these changes
Jun 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A followup to #2737 where, while I was in the code, I saw a couple of things that could be cleaned up (in terms of taking on newer approaches we've adopted) and also the chance to eliminate what very much looks like a wee bit of dead code (#2742).
On the latter point, as mentioned in the linked issue that I'd made as reminder for myself, all evidence suggests that that code was handling a situation that seems to no longer be possible in Textual (seeing keys bound to an action in an
on_keyhandler). Removing this code makes zero difference to theInputunit tests, and hand-testing (in the demo for example, where we have plain and also password inputs) shows zero ill-effects too.The other tweak here is to make the watchers "private". Once #2708 lands there's likely a couple of similar changes can be made elsewhere here too.