Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pilot cannot press key minus #1815

Closed
rodrigogiraoserrao opened this issue Feb 16, 2023 · 6 comments · Fixed by #1926
Closed

Pilot cannot press key minus #1815

rodrigogiraoserrao opened this issue Feb 16, 2023 · 6 comments · Fixed by #1926
Assignees
Labels
bug Something isn't working Task

Comments

@rodrigogiraoserrao
Copy link
Contributor

rodrigogiraoserrao commented Feb 16, 2023

The way App._press_keys is implemented is flawed in here:

textual/src/textual/app.py

Lines 830 to 842 in ba6c8af

if len(key) == 1 and not key.isalnum():
key = (
unicodedata.name(key)
.lower()
.replace("-", "_")
.replace(" ", "_")
)
original_key = REPLACED_KEYS.get(key, key)
char: str | None
try:
char = unicodedata.lookup(original_key.upper().replace("_", " "))
except KeyError:
char = key if len(key) == 1 else None

There is a clear bug because the transformation done on key with two consecutive replacements cannot be undone in line 840 with a single replacement.
In particular, the keys HYPHEN-MINUS and REVERSE SOLIDUS show that the underscores that we get after the two consecutive replacements cannot be unambiguously reverted.

On top of that, I have to understand what is the issue with a key that has length 1 and is not "alnum".
For example, "-" has length 1, is not "alnum", and could very well be pressed as is. Maybe we want to check if key.isprintable instead?

@rodrigogiraoserrao rodrigogiraoserrao added bug Something isn't working question Further information is requested Task labels Feb 16, 2023
@rodrigogiraoserrao rodrigogiraoserrao self-assigned this Feb 16, 2023
@rodrigogiraoserrao
Copy link
Contributor Author

rodrigogiraoserrao commented Feb 17, 2023

#1830 reveals another instance of the problematic fake restoration of the original name:

upper_original = original_key.upper().replace("_", " ")

@ofek
Copy link
Contributor

ofek commented Feb 17, 2023

I wonder if that explains these intermittent flakes when entering a temporary directory into an Input:

>               assert app.config.data['repos'] == {'agent': {'path': path}}
E               AssertionError: assert {'agent': {'p...\tmp8rutnaj'}} == {'agent': {'p...tmp8rut_naj'}}
E                 Differing items:
E                 {'agent': {'path': 'C:\\Users\\ofek\\AppData\\Local\\Temp\\tmp8rutnaj'}} != {'agent': {'path': 'C:\\Users\\ofek\\AppData\\Local\\Temp\\tmp8rut_naj'}}
E                 Use -v to get more diff

C:\Users\ofek\Desktop\code\ddqa\tests\screens\test_configure.py:290: AssertionError
---------------------------------------------------------------------------------------- Captured stdout call ----------------------------------------------------------------------------------------
press 'C' (char='C')
press 'colon' (char=':')
press 'reverse_solidus' (char='\\')
press 'U' (char='U')
press 's' (char='s')
press 'e' (char='e')
press 'r' (char='r')
press 's' (char='s')
press 'reverse_solidus' (char='\\')
press 'o' (char='o')
press 'f' (char='f')
press 'e' (char='e')
press 'k' (char='k')
press 'reverse_solidus' (char='\\')
press 'A' (char='A')
press 'p' (char='p')
press 'p' (char='p')
press 'D' (char='D')
press 'a' (char='a')
press 't' (char='t')
press 'a' (char='a')
press 'reverse_solidus' (char='\\')
press 'L' (char='L')
press 'o' (char='o')
press 'c' (char='c')
press 'a' (char='a')
press 'l' (char='l')
press 'reverse_solidus' (char='\\')
press 'T' (char='T')
press 'e' (char='e')
press 'm' (char='m')
press 'p' (char='p')
press 'reverse_solidus' (char='\\')
press 't' (char='t')
press 'm' (char='m')
press 'p' (char='p')
press '8' (char='8')
press 'r' (char='r')
press 'u' (char='u')
press 't' (char='t')
press 'n' (char='n')
press 'a' (char='a')
press 'j' (char='j')

@rodrigogiraoserrao
Copy link
Contributor Author

rodrigogiraoserrao commented Feb 17, 2023

Edit: Ignore this message.

It could be related. Notice that there is a character missing from the logged presses and it corresponds to the underscore, for which unicodedata.name gives one of those weird names:

>>> import unicodedata
>>> unicodedata.name("_")
'LOW LINE'

However, without further investigation I can't tell.
Can you give me a bit more context into the code block with a failing test that you shared?

@rodrigogiraoserrao
Copy link
Contributor Author

There are contexts in which _ is used to denote a pause, which is problematic if what you really want is the underscore character!
How do you specify those key presses into the Input? If you replace a _ with underscore, does it work @ofek ?

@ofek
Copy link
Contributor

ofek commented Feb 18, 2023

That works, thank you!!!

@rodrigogiraoserrao rodrigogiraoserrao removed the question Further information is requested label Feb 27, 2023
rodrigogiraoserrao added a commit that referenced this issue Mar 2, 2023
Throughout the code base, there are a couple of places (noted in #1830) where we take a Unicode name and make it more friendly, or take a 'friendly' name and try to recover the original Unicode name. We add a function to go from 'friendly' to the Unicode name, which tackles the issue highlighted in #1815.

Related issues: #1815, #1830.
@github-actions
Copy link

github-actions bot commented Mar 6, 2023

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants