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 double-byte, if needed multi supported same way #72

Closed
wants to merge 1 commit into from

Conversation

liuzheng
Copy link

@liuzheng liuzheng commented Jun 4, 2015

The red line is original window size col=80
After clicked the resize Button , col changed to 100
so you can see
my test env is Mac+chrome maybe it different from others, I think just a little

image

data = line[i][0];
ch = line[i][1];

if (i === x) data = -1;

if (ch.charCodeAt()>=127){widthd=widthd-0.7}

Choose a reason for hiding this comment

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

I'm pretty sure this should not be every character over 127 - there are many characters beyond that which should still be displayed as single width, like £ (163), or ø (248). Those are just examples, there are many, many more, and working out which ones need width adjustment is tricky.

I'm also not sure where the 0.7 comes from. This may work for the wide characters of one particular font, but I wouldn't rely on it working in general.

There is already a mechanism for dealing with wide characters, which works by allowing them twice the normal space in the grid. However, it doesn't know about all the wide characters, as described in #66.

Copy link
Author

Choose a reason for hiding this comment

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

127 and 0.7 also my empirical data, so maybe just fit for me

thanks for your work!

@chjj
Copy link
Owner

chjj commented Sep 12, 2015

The isWide() check was a naive implementation. It was never meant to work entirely properly.

This will not work until the unicode module from blessed is ripped out and placed here. This is a very complex problem and does not only deal with double-width chars, but also surrogate pairs, combining chars, combining chars that are surrogates (2 in length, 0 in width), etc. It's almost ridiculous how complicated this issue is. It's a miracle we got it working at all in blessed. It also gives a huge hit to performance, even if double-width/combining/surrogates are not being used. It makes things slower just by existing. utf-16 was a major mistake on the part of javascript. By the same token, I now have contempt for double-width characters. I spend dozens of hours getting them implemented for blessed in a way that wouldn't slow everything drastically.

This looks like a quick fix, but it will break in a number of situations. I'm not sure what the significance is of checking whether a character is non-ascii either.

Closing until an actual suitable solution is found. The solution is in blessed, but if we want to implement it in term.js, we have to work backwards since blessed renders to the terminal, and term.js is the terminal. We'll be doing the inverse.

@chjj chjj closed this Sep 12, 2015
@yoshiokatsuneo
Copy link

I tried to made a generic solution for all the non-english characters.
#97

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.

None yet

4 participants