Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

fix(cursor): fix terminal cursor move display #180

Closed
wants to merge 2 commits into from
Closed

fix(cursor): fix terminal cursor move display #180

wants to merge 2 commits into from

Conversation

mritd
Copy link
Contributor

@mritd mritd commented Feb 28, 2019

fix the problem of moving the cursor display when the terminal inputs non-English characters (such as Chinese, Japanese)

refs #126

Signed-off-by: mritd mritd1234@gmail.com

fix the problem of moving the cursor display when the terminal inputs non-English characters (such as Chinese, Japanese)

refs #126

Signed-off-by: mritd <mritd1234@gmail.com>
@mritd
Copy link
Contributor Author

mritd commented Feb 28, 2019

I fixed the cursor display problem using the https://github.com/mattn/go-runewidth library,
but I don't know how to write a complete test case to test it...

I did a lot of testing locally, now it looks good work 😂

2019-02-28 22 41 04

@AlecAivazis
Copy link
Owner

AlecAivazis commented Mar 5, 2019

Thanks for submitting this @mritd!

I tried using 👫as an input and got some weird behaviors if I moved around and tried to delete it. I haven't yet taken the time to investigate if this is a problem with the go-runewidth or something in here but I thought I would raise it to you.

@mritd
Copy link
Contributor Author

mritd commented Mar 6, 2019

@AlecAivazis Have you encountered a problem with the cursor suddenly appearing on the next line?

@mritd
Copy link
Contributor Author

mritd commented Mar 6, 2019

I found in the test: When pressing the left arrow (move button) and the delete button at the same time, there is a certain probability that the cursor will suddenly appear on the next line.

2019-03-06 22 30 36

I was testing the master branch code (not merging this PR); I guess this may be due to a location variable thread is not safe.

merge upstream

Signed-off-by: mritd <mritd1234@gmail.com>
@AlecAivazis
Copy link
Owner

@mritd any luck hunting this down? There are a bunch of changes that have been merged recently so you probably want to open a new PR based on the current master

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants