Skip to content

Fixes #5057. Pressing Alt-T in a TextField causes a t to be entered#5059

Merged
tig merged 1 commit intogui-cs:developfrom
BDisp:5057_kytty-alt-with-printable-key-fix
Apr 23, 2026
Merged

Fixes #5057. Pressing Alt-T in a TextField causes a t to be entered#5059
tig merged 1 commit intogui-cs:developfrom
BDisp:5057_kytty-alt-with-printable-key-fix

Conversation

@BDisp
Copy link
Copy Markdown
Collaborator

@BDisp BDisp commented Apr 22, 2026

Fixes

Proposed Changes/Todos

  • Invalidade AssociatedKey if Ctrl or Alt exist

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

@BDisp BDisp requested a review from tig as a code owner April 22, 2026 20:38
@tig
Copy link
Copy Markdown
Collaborator

tig commented Apr 22, 2026

I've already fixed this as part of #5062

@tig tig closed this Apr 22, 2026
@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Apr 22, 2026

I've already fixed this as part of #5062

The fix would only be in KittyKeyboardPattern.cs and not in TextField, TextView, or any other view. I hope you reconsider this because if there's another view that needs to handle Alt+Letter, you'll have to repeat the same code everywhere.

@tig
Copy link
Copy Markdown
Collaborator

tig commented Apr 23, 2026

First, my apologies for being so terse and closing this abruptly. I was in a hurry. Then I was with my wife and pissed at myself for not taking the time to explain and discuss with you.

@tig
Copy link
Copy Markdown
Collaborator

tig commented Apr 23, 2026

I kinda like the idea of having associated text be available even if it's an alt press.

If you study my fix you'll see the fixed impls (textfield, textview, hexview) all failed to check for alt/ctrl properly before.

They had bugs that were latent because before there was no concept of associated character. But with that they were blindly consuming alt keys.

So in think my fix is Good (capital G).

Your fix is at a much lower level, but prevents the key info from passing all the way up the stack. It doesn't negate my fix, but just means those classes are now defensive in a case where they probably don't have to be.

So the point I'd like to debate with you is whether it is correct for associatedchar to be carried with ALL keystrokes or not. I asset it is because that's consistent.

What are your thoughts?

@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Apr 23, 2026

No problem. Although my solution suppress the associated key you still have access through the Key.KeyCode. In my opinion the Key.AsGrapheme should only have a value when it's a printable key. So the Alt+keys or Ctrl+keys shouldn't be printable keys but only shortcuts. Can you please say the cases where you need them on the AsGrapheme and on the AsRune instead of having them on the KeyCode?

@tig
Copy link
Copy Markdown
Collaborator

tig commented Apr 23, 2026

No, I can't.

Let's go with both fixes. Mine makes the implementations more robust.

Yours makes the pipeline correct.

@tig tig reopened this Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pressing Alt-T in a TextField causes a t to be entered

2 participants