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

Align keyboard and mouse support in FullscreenRenderer with LightRenderer #2616

Merged
merged 7 commits into from
Oct 2, 2021

Conversation

vovcacik
Copy link
Contributor

This pull request is focused on fixing some issues and adding new features in the FullscreenRenderer. Most of the branch is about keyboard events, but the last commit also implements mouse events.

Keyboard

  • I tried to bring the keyboard events in FullscreenRenderer as close as possible to the LightRenderer
  • additions:
    • extensive testing of the tcell events that fzf uses as source for keyboard events
    • ability to input AltGr characters on foreign keyboards
  • fixes:
    • CtrlSpace event didn't work
    • CtrlCaret event wasn't implement
    • there was big discrepancy in how backspace key is handled between FullscreenRenderer and LightRenderer. The CtrlH event was emitted even in the cases where BSpace or AltBS was appropriate.

Mouse

The mouse events in LightRenderer are consumed by the shell in Windows, so I didn't have reference point. Based on existing code it should be close to what user would expect, but I noticed something that looked like Shift+Click support in LightRenderer, so this implementation may still lag behind the unix one.

@junegunn
Copy link
Owner

Thanks. I'll review the patch when I get some time.

/cc @kelleyma49

This contains one test case of each tcell.Key* event type that can be
sent to and subsequently processed in fzf's GetChar(). The test cases
describe status quo, and all of them PASS.

Small function util.ToTty() was added. It is similar to util.IsTty(),
but for stdout (hence the To preposition).
CtrlH events are still sent when appropriate. I have adjusted
FullscreenRenderer to match the LightRenderer's behaviour, which seems
to be correct.
src/tui/tui.go Outdated Show resolved Hide resolved
To prevent including tcell library in non-windows builds.
@vovcacik
Copy link
Contributor Author

vovcacik commented Oct 1, 2021

I am bit puzzled by the failing integration check. I've rerun the workflow on the fork with same result though.

@junegunn junegunn merged commit b8aa2d2 into junegunn:master Oct 2, 2021
@junegunn
Copy link
Owner

junegunn commented Oct 2, 2021

Merged, thanks for the contribution!

@vovcacik vovcacik deleted the fsrender_fixes branch October 5, 2021 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants