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

Fix invoking application mode after leaving it (Fixes #650) #1149

Closed
wants to merge 2 commits into from

Conversation

JaCoB1123
Copy link

@JaCoB1123 JaCoB1123 commented Aug 3, 2023

According to terminfo:
smkx is enter application mode and rmkx is leave back to normal mode

Thanks @tpoliaw

@vercel
Copy link

vercel bot commented Aug 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
atuin-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2023 8:48pm

@tpoliaw
Copy link
Contributor

tpoliaw commented Aug 3, 2023

This is what I tested locally and it solved the problem for me but I wasn't sure whether it'd cause issues elsewhere. The comment Switch to cursor mode, then back to application suggests that the current way around was intentional and that it should be left in application mode.

Either way, if this PR is ok, the comment should be removed/updated so they're not out of sync.

@JaCoB1123
Copy link
Author

I just updated the comments to match the code.

@ellie I saw you added this in #31 / 4b40527. Do you by any chance still know why it was added?

@ellie
Copy link
Member

ellie commented Aug 7, 2023

I just updated the comments to match the code.

@ellie I saw you added this in #31 / 4b40527. Do you by any chance still know why it was added?

I'll have to double check before merging this. I remember there were issues with certain keycodes not being recognized in application mode, but that's probably been resolved by now.

(also... can't believe it's been >2yrs 😭)

ellie added a commit that referenced this pull request Aug 14, 2023
This was initially in place for when we used a different terminal
backend. That backend required that the terminal be in a specific mode,
or otherwise key modifiers would not be correctly recognized. It was
super frustrating.

Since the move to crossterm, we automatically switch to raw mode: https://github.com/atuinsh/atuin/blob/b48de9bd9d89fb9b6a0044a1b251e5b2ff116387/atuin/src/command/client/search/interactive.rs#L528

Should resolve #987, #650, #909, #492

Should also supercede #1149
ellie added a commit that referenced this pull request Aug 14, 2023
This was initially in place for when we used a different terminal
backend. That backend required that the terminal be in a specific mode,
or otherwise key modifiers would not be correctly recognized. It was
super frustrating.

Since the move to crossterm, we automatically switch to raw mode: https://github.com/atuinsh/atuin/blob/b48de9bd9d89fb9b6a0044a1b251e5b2ff116387/atuin/src/command/client/search/interactive.rs#L528

Should resolve #987, #650, #909, #492

Should also supercede #1149
@ellie
Copy link
Member

ellie commented Aug 14, 2023

Hey! I looked into this this morning, and I don't think we actually need the mode switching any more. I've removed it in #1170

@ellie ellie closed this Aug 14, 2023
@JaCoB1123 JaCoB1123 deleted the fix-zsh-search branch August 14, 2023 12:42
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.

3 participants