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

doubled search inputs on Windows #142

Open
farnoy opened this issue Jul 2, 2024 · 8 comments
Open

doubled search inputs on Windows #142

farnoy opened this issue Jul 2, 2024 · 8 comments

Comments

@farnoy
Copy link

farnoy commented Jul 2, 2024

Describe the bug
I'm using minus through jj/jujutsu. It works fine overall except for search. ? (backwards search) does not engage at all and / (forward search) suffers from double inputs. When I press / once, I already get a forward slash as the first character to search for. When I press any character after that, I get it inputted twice.

To Reproduce

  1. Press /, then any character, like a
  2. Observe the bottom status line be //aa (a forward search for /aa

Expected behavior

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop:

  • Windows 11
  • alacritty 0.13.2 (bb8ea18)
  • powershell
  • minus 5.6.1 from jj

Additional context

@AMythicDev
Copy link
Owner

I am unable to reproduce this on Fedora machine. I also tried on a Ubuntu container and still cannot reproduce it (all using alacritty). So it seems its likely a Windows only bug. I will continue investigating this...

@AMythicDev
Copy link
Owner

I tried running one of the examples but can't reproduce it even on Windows 11 + Powershell (not alacritty though) so I am not sure where this is coming from. Since I am using a VM the VS debugger is incredibly slowwwwww.... Can you use a debugger and try to find where this originates from?

@farnoy
Copy link
Author

farnoy commented Jul 2, 2024

I tried reproducing on $ cargo run --example static --features=static_output,search but it has different bugs:

  1. backwards search ? does not engage at all
  2. previous result with p does not work at all
  3. after entering forwards search /, the first character doesn't get registered ~half the time. eg. /55 ends up with a /5 search

Can you reproduce any of these?

@AMythicDev
Copy link
Owner

AMythicDev commented Jul 2, 2024

Haven't tested 1 and 2 but can get 3. My latest commit fixed 3 on my Linux box but it seems to occur on Windows.

@AMythicDev
Copy link
Owner

Alright after some through investigation on 3 and I found out the following:-

  1. On Linux/MacOS/other UNIX-like OSs at the search prompt for each key press crossterm reports a single KeyEvent with kind as KeyEventKind::Press. On Windows, it reports a pair of KeyEvent: one with kind KeyEventKind::Press and the next with kind KeyEventKind::Release.
  2. On Windows when starting a search with /, minus reads the first of the key event with kind press and ignores the other half with kind release which causes it to re-read when the search prompt starts.
  3. Now when the first key is pressed after the search prompt is open say k, the release half of the / overlaps the press half and hence nothing happens.

I think this is more of crossterm issue rather than minus. It appears due to the way crossterm handles events on Windows and hence it should be fixed on crossterm itself.

Sorry I don't own a proper Windows setup so its hard for me to do more investigation of crossterm on Windows. If you have a Windows machine, please report this to the crossterm devs.

Also I will have a fix for 1 and 2 soon.

@farnoy
Copy link
Author

farnoy commented Jul 15, 2024

I'm not familiar with crossterm and don't know if you've seen it already, but there's crossterm-rs/crossterm#778 already. It sounds like it may solve at least some of the issues identified here?

@farnoy
Copy link
Author

farnoy commented Jul 15, 2024

That issue also leads to #141. I could try a full end to end test by integrating the latest minus into jj to see which issues remain (if any).

@farnoy
Copy link
Author

farnoy commented Jul 15, 2024

OK, testing on 43aa50d, they're not doubled anymore, but they:

  1. Feel debounced/delayed - I think it tries to repaint after every character which can slow down perceived input
  2. Can't do capital letters anymore - Shift+A does not input anything

As for the previous issues: ? doesn't work, likely due to the shift modifier. n/p for next/previous search result work but p does not scroll the window up - I can see the occurrence number decrementing in the bottom right corner, but the window is not scrolled up.

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

No branches or pull requests

2 participants