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

ui: prompt: Better unicode support #295

Merged
merged 3 commits into from
Jun 20, 2021
Merged

ui: prompt: Better unicode support #295

merged 3 commits into from
Jun 20, 2021

Conversation

archseer
Copy link
Member

We copied over eval_movement from wezterm, that already solves most of
our problems. self.cursor is now byte-based.

Fixes #293

We copied over eval_movement from wezterm, that already solves most of
our problems. self.cursor is now byte-based.
@archseer archseer requested a review from CBenoit June 18, 2021 08:32
helix-term/Cargo.toml Outdated Show resolved Hide resolved
@archseer
Copy link
Member Author

Should make for an easier implementation of other keys too (ctrl-k, etc)

@pickfire
Copy link
Contributor

So now we should be able to type unicode in prompt right?

Copy link
Contributor

@pickfire pickfire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. At least unicode works now. But I think we should add the keys to use the functions.

Why not just import the original LineEditor?

Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and I couldn't break it.
Cursor is working perfectly even with Japanese
image

@archseer
Copy link
Member Author

Added a bunch more commands including ctrl-k, alt-b,alt-f, ctrl-b, ctrl-f and home/end.

Comment on lines +44 to +47
BackwardChar(usize),
BackwardWord(usize),
ForwardChar(usize),
ForwardWord(usize),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we support repetition for prompt since we only need one. I think we might be able to remove the usize here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll remove it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah wezterm also always uses 1, I guess it was added since you could do vi-mode for readline in the future with it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but I don't think we will be doing that any time soon right? At least I think vi-mode won't be useful in parts like prompt where a modal editor is not useful.

ForwardWord(usize),
StartOfLine,
EndOfLine,
None,
Copy link
Contributor

@pickfire pickfire Jun 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what case do we need this None?

@archseer
Copy link
Member Author

Going to merge this as is, None will be used shortly for certain types of kill where we calculate the delete and cursor move with different movements. I'm also keeping count because it's simpler to keep in sync with the termwiz implementation for now.

@archseer archseer merged commit 34ebe82 into master Jun 20, 2021
@archseer archseer deleted the prompt-redux branch June 20, 2021 07:38
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.

Floating cursor in command prompt when inputing unicodes
3 participants