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

The VI-mode commands f' , f" , t' and t" insert ' or " instead of moving the cursor when running on Windows when using the International Layout (dead keys) #3795

Open
3 tasks done
thomazmoura opened this issue Sep 7, 2023 · 12 comments
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-More Investigation Initial investigation is done, but need to follow up. VI-Mode

Comments

@thomazmoura
Copy link

Prerequisites

  • Write a descriptive title.
  • Make sure you are able to repro it on the latest released version
  • Search the existing issues, especially the pinned issues.

Exception report

N/A

Screenshot

PowerShell VI Mode Windows bug
Print showing that when I try to use F" or F' it prints the character instead of moving the cursor to the previous ocurrence of the character.

Environment data

PS Version: 7.2.13
PS HostName: ConsoleHost
PSReadLine Version: 2.2.6
PSReadLine EditMode: Vi
OS: 10.0.19041.320 (WinBuild.160101.0800)
BufferWidth: 120
BufferHeight: 9001

Steps to reproduce

(The following steps can be made replacing ' for " or `)

  • Set the Windows language layout to United States-International
  • Write anything with an ' on it on PowerShell
  • Press ESC to go the VI-mode's normal mode
  • Try to move to the written character with f' or t' (or if the character is to the left of the cursor with F' or T')

(the key here is used to send the ' key directly instead of merging it with another key since in this layout it's a dead key)

Expected behavior

The cursor should move to the desired character, exactly like it happens on the US layout (the only difference should be the need to press space after ' or " or `).

Actual behavior

When running it on WSL2 it works exactly as expected - since it's a dead key on US International Layout when I press f " it waits the next key and if I press space (to insert the " character) it searches for next ocorrence of the " character. But when running on Windows it doesn't seem to recognize the dead key and fails the command as soon as I press any of it.

I've noticed it sends a bell as soon as I press the dead key (if I press d f and then ' as soon as the ' is first pressed it sounds the bell), which I believe indicates that the previous command failed/was cancelled and then when I press space it prints the character (if I press the same accent again it prints it twice) which would be the expected behaviour if I was on insert mode and not chaining commands on normal mode.

I've made tests with the US keyboard layout and it works fine. Also, some accented words like à or á (that are written with the keys a backtick and a ' trigger the same behavior, just printing the character instead of moving the cursor.

As an additional info the ^ key is also a dead key on this layout but has a different behaviour - it doesn't ring any bell, and when chained to command it just ignores the command and goes to the start of the line (the expected behaviour when the command is like d^ would for it to delete until the start of the line, not just go there).

I've also made many tests like using Windows Terminal or conhost and using both PowerShell 5 and PowerShell 7 versions (with the latest preview of PSReadLine). So far it only happens on Windows and when using the United States-Internatial layout (which is the one I use to be able to write both english and portuguese on my ANSI layout keyboard)

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Triage 🔍 It's a new issue that core contributor team needs to triage. label Sep 7, 2023
@thomazmoura
Copy link
Author

Just tested on VS Code and it works there, using the same PowerShell and PSReadLine versions.

Also, I've realized I wasn't using the latest pre-release version of PSReadLine and I've just updated it - but on Windows Terminal and conhost the problem persists even on 2.3.2 (beta2)

@thomazmoura
Copy link
Author

Here's a exception report from a related issue that happens from time to time but I still haven't managed to discover exactly when (it always happens after trying to go to a ' or a ", realizing it won't work on Windows and trying to workaround moving to another character instead (usually a -).

The command I was using is Criar-PullRequest.ps1 -Titulo 'Implementação de criação de campanhas e formulário de pesquisa' -Equipe Arquitetura -AbrirPullRequest (it's a custom script I have) - I was trying to change all the text from the first ' to the second ' so I could rerun the command with a different text. When it failed, I tried changing to the - character and then the exception was thrown.


Oops, something went wrong.
Please report this bug with ALL the details below, including both the 'Environment' and 'Exception' sections.
Please report on GitHub: https://github.com/PowerShell/PSReadLine/issues/new?template=Bug_Report.yaml
Thank you!

Environment

PSReadLine: 2.3.2-beta2
PowerShell: 7.2.13
OS: Microsoft Windows 10.0.19045
BufferWidth: 144
BufferHeight: 40

Last 200 Keys:

p s 1 Spacebar - T i t u l o Spacebar ' I m p l e m e n t a ç ã o Spacebar d e Spacebar c r i a ç ã o Spacebar d e Spacebar c a m p a n h a s Spacebar e Spacebar f o r m u l á r i o Spacebar d e Spacebar p e s q u i s a ' Spacebar - E q u i p e Spacebar A r q u i t e t u r a Spacebar - A b r i r P u l l R e q u e s t Escape I # Enter # End Escape Shift+ ^ x Enter Escape k f ' h x f - ; ; ; , , l l l l l l l l l l l h h c t ' h x f - F ' h x h h h h h h h h h h h h h h h h h h h h h h h h h B B B B B B l c t -

Exception

System.InvalidOperationException: Operation is not valid due to the current state of the object.
at Microsoft.PowerShell.PSConsoleReadLine.StartEditGroup()
at Microsoft.PowerShell.PSConsoleReadLine.GroupUndoHelper.StartGroup(Action2 instigator, Object instigatorArg) at Microsoft.PowerShell.PSConsoleReadLine.ViReplaceToBeforeChar(Char keyChar, Nullable1 key, Object arg)
at Microsoft.PowerShell.PSConsoleReadLine.ViReplaceToBeforeChar(Nullable1 key, Object arg) at Microsoft.PowerShell.PSConsoleReadLine.ProcessOneKey(PSKeyInfo key, Dictionary2 dispatchTable, Boolean ignoreIfNoAction, Object arg)
at Microsoft.PowerShell.PSConsoleReadLine.ViChordHandler(Dictionary2 secondKeyDispatchTable, Object arg) at Microsoft.PowerShell.PSConsoleReadLine.ViChord(Nullable1 key, Object arg)
at Microsoft.PowerShell.PSConsoleReadLine.ProcessOneKey(PSKeyInfo key, Dictionary2 dispatchTable, Boolean ignoreIfNoAction, Object arg) at Microsoft.PowerShell.PSConsoleReadLine.InputLoop() at Microsoft.PowerShell.PSConsoleReadLine.ReadLine(Runspace runspace, EngineIntrinsics engineIntrinsics, CancellationToken cancellationToken, Nullable1 lastRunStatus)

@StevenBucher98 StevenBucher98 added Needs-More Investigation Initial investigation is done, but need to follow up. VI-Mode and removed Needs-Triage 🔍 It's a new issue that core contributor team needs to triage. labels Sep 25, 2023
@StevenBucher98 StevenBucher98 self-assigned this Sep 25, 2023
@StevenBucher98
Copy link
Collaborator

StevenBucher98 commented Sep 25, 2023

Interesting bug, able to reproduce and also seems to work with Mac OS Terminal. @DHowett any ideas if this is an issue with Windows Terminal/conhost? (using latest 2.3.3 version of PSRL)

@StevenBucher98 StevenBucher98 added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage 🔍 It's a new issue that core contributor team needs to triage. labels Sep 25, 2023
@StevenBucher98 StevenBucher98 added Needs-Author Feedback Needs-More Investigation Initial investigation is done, but need to follow up. Area-Accessibility Label for issues related to accessibility problems or improvements and removed Needs-Triage 🔍 It's a new issue that core contributor team needs to triage. Needs-More Investigation Initial investigation is done, but need to follow up. Needs-Author Feedback Area-Accessibility Label for issues related to accessibility problems or improvements labels Oct 9, 2023
@thomazmoura
Copy link
Author

I've just tested on 2.3.4 (though the comment asking for this seem to have disappeared) and the behavior seem to be the same still. It's like when I press a dead key it "counts" already as part of the combo, but it's not recognized so it breaks the chain. When I press space (or any other key for that matter) to actually send the proper character the chain is already broken so it inserts the character normally like there was no command at all.

I believe the way to fix it is probably ignoring the dead key presses for the command chaining (which seems to be what happens on Linux, for example), but I have no idea how to do that or where the code for this command chaining is.

I might be able to take a look if there is any documentation or instructions on where in the code that part is processed.

@StevenBucher98 StevenBucher98 removed their assignment Nov 3, 2023
@thomazmoura
Copy link
Author

thomazmoura commented Jan 13, 2024

@StevenBucher98, could you help me run the PSReadLine locally or point to some documentation or tutorial for it? I've tried building the PSReadLine project, starting a new PowerShell (pwsh) session on the bin/Debug/net6.0 folder, Removing the pre-loaded PSReadLine and then importing the PSReadLine.psm1 file, but it doesn't seem to load any the changes I made (currently I'm just throwing some exceptions to garantee that I'm running the changed code and not the default one). I've also tried running Import-Module .\Microsoft.PowerShell.PSReadLine2.dll as suggested by copilot, but it gives the error Import-Module: Assembly with same name is already loaded. I found no way to start a new pwsh session without PSReadLine pre-imported so I could import the local version.

Just to give an update I've so far found that the code which is likely the culprit is the SearchChar method on ReadLine.vi.cs (line 166). There it calls ReadKey() and after getting the value proceeds to make the movement. The part where key is actually read is not shown on the ReadKey() method (it seems to happen on another thread) but my guess is that it runs something similar to calling [Console]::ReadKey() on pwsh and that's where Windows and Linux diverge considerably. On Linux, any dead keys are ignored by [Console]::ReadKey() - only when the next character is typed it returns the combined character (like á, or ã) that way on layouts that have ', " or ` as dead keys it waits for a another key (like space) to return the key. On windows though, this same method returns immediately when a dead key is typed, but with a null/empty KeyChar. The PSReadLine reads this empty KeyChar and then does nothing. When the user finally types the inteded key the chain has already been broken by the dead key.

Since resolving this on the dotnet side seems way more troublesome (since any change there can impact the whole ecosystem and there many low level considerations on these changes) and this bug is currently affecting only the VI mode on PSReadLine for layouts with dead keys, what I would like to attempt is to have a loop on the ReadKey() (or wrapped around on the ReadKey in places like the SearchChar method and similar methods) so that when a user presses a dead key PSReadLine continues waiting or ignore it and then call ReadKey again, until a valid key is pressed. But to do this I need a way to run PSReadLine locally.

thomazmoura added a commit to thomazmoura/PSReadLine that referenced this issue Jan 25, 2024
@thomazmoura
Copy link
Author

@StevenBucher98 I've managed to be able to test locally (basically all I needed was pwsh -NoProfile -NonInteractive).
Then I found the issue made a PR (#3928) could you help me getting someone to review it?

@matteocoder
Copy link

@thomazmoura thank you for looking into this. This issue with the dead keys manifests itself even in emacs and windows modes with the functions CharacterSearch and CharacterSearchBackward. For me, the best solution long-term would be to fix the buggy ReadKey() implementation on windows.

@thomazmoura
Copy link
Author

I do agree that this would definitely be the best solution - I'm just not sure when something like this will be done and the amount of effort needed, since the problem with the ReadKey() implementation seems to come from .NET and not even from PowerShell. And I felt it would make sense to have this kind of thing be handled on PSReadLine in the end - at least if it were easier to identify dead keys specifically.

That said, I do realize it's likely a hard decision to add a fix on a code that will run for everyone when only people using dead keys on Windows would be affected by this, at least in the way I managed to fix it - if there way a sure way to recognize that the pressed key was a dead key and not a function key, for exemple, I would argue that the fix made perfect sense, since there is no situation on which a dead key would be a valid key for this movements.

Could we at least have a workaround for the time being? I would be happy to use my own customized version of PSReadLine until this is fixed on .NET, but I couldn't setup an easy way to overwrite the built-in PSReadLine. To test I had to open a new PowerShell window with -NonInteractive and manually import the custom one. I think I'll try later to put on the Windows Terminal profile a command to open pwsh -NonInteractive with a custom powershell profile that imports the custom PSReadLine and then loads my default powershell profile. That's far from ideal, but might be better than this eventual breaks on my flow because I forgot that on Windows this basic things don't work.

@matteocoder
Copy link

matteocoder commented Jun 26, 2024

@thomazmoura I share your frustration… Since you have already working code, could you extend your workaround to work in Emacs and Windows modes as well?

In the meantime, may I open an issue in the dotnet issue tracker about ReadKey() and dead keys, and quote your findings in the issue's body?

@thomazmoura
Copy link
Author

thomazmoura commented Jun 27, 2024

My workaround was to create a new function ReadKeyChar() (it basically has a loop until a character that has an actual KeyChar is pressed) and change every reference of ReadKey().KeyChar to ReadKeyChar() on PSReadLine/ReadLine.vi.cs.

I suppose that doing the same on the files related to the emacs binding and window modes would work as well - though I admit I don't know them well so I can't garantee.

And if you're willing to open an issue there, please, do it. I admit I did not open it myself because there were already some ongoing discussions about ReadKey on Linux vs Windows (like the 96490 issue on the runtime repo) and I found it hard to even figure out if the dead keys issue was included on the discussions or not (I've also took a look at the code, but there's just too much low level details on each platform implementation for me to follow).

@thomazmoura
Copy link
Author

@thomazmoura I share your frustration… Since you have already working code, could you extend your workaround to work in Emacs and Windows modes as well?

In the meantime, may I open an issue in the dotnet issue tracker about ReadKey() and dead keys, and quote your findings in the issue's body?

In case you want to try it out, the way I managed to overwrite PSReadLine locally on Windows is to start pwsh with the -NonInteractive flag and add two imports on my profile file ($PROFILE) changing [PSReadLineRepoFolder] for the complete path to the cloned and tweaked PSReadLine repo:

Import-Module [PSReadLineRepoFolder]\PSReadLine\PSReadLine.psm1
Import-Module [PSReadLineRepoFolder]\PSReadLine\bin\Debug\net6.0\Microsoft.PowerShell.PSReadLine2.dll

Then I configured Windows Terminal profile to open by default the pwsh with this flag: "commandline": "pwsh -NonInteractive",

That way whenever I open Windows Terminal it loads the profile with pwsh without the PSReadLine attached and then imports the local version with the fix. Quite a stretch to just make powershell work as well on Windows as it does on WSL, but I'm afraid this will have to do until they fix it.

@matteocoder
Copy link

Thank you so much for your explanation, and for your work. I hope a fix will be implemented soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-More Investigation Initial investigation is done, but need to follow up. VI-Mode
Projects
None yet
Development

No branches or pull requests

3 participants