-
Notifications
You must be signed in to change notification settings - Fork 740
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
Improved unicode wcwidth support #1001
base: master
Are you sure you want to change the base?
Conversation
src/util/chwidth.cc
Outdated
|
||
int ChWidth::chwidth( wchar_t wc ) const | ||
{ | ||
if (static_cast<size_t>( wc ) > widths.size()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>=
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching that.
src/unicode/Makefile.am
Outdated
git submodule init | ||
git submodule update | ||
|
||
ucd/PropList.txt ucd/UnicodeData.txt ucd/EastAsianWidth.txt: UCD.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This target will run multiple times concurrently and race with itself when building with -j
.
src/unicode/Makefile.am
Outdated
unzip -o -d ucd UCD.zip | ||
ln -fs ucd/PropList.txt ucd/UnicodeData.txt ucd/EastAsianWidth.txt . | ||
|
||
eaw_narrow_map eaw_wide_map: ucd/PropList.txt ucd/UnicodeData.txt ucd/EastAsianWidth.txt edit_widths.py make_widths.py google-libapps/libdot/third_party/wcwidth/ranges.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both fixed in the rebase coming momentarily.
But they’re still expanded out to |
This code adds a system-independent Unicode widths table to Mosh, and adds a scheme for the client to propagate local configuration to the server. We also generate character width tables from Unicode data. This brings in Google libapps as a Git submodule. This uses Unicode 9.0 as the base map for demo purposes. The default map is 11.0. It also adds an --eaw-wide option to select wide/narrow for East Asian Ambiguous Width characters.
This hold/release is unnecessary because mosh-client always sends all info needed for the session in its first message, and mosh-server holds the session until after receiving the first message. But this does test the server-side functionality somewhat.
No, I've not decided anything about the final implementation of the actual lookup, or done any implementations/benchmarking/etc yet. Better seemed the enemy of good enough here, and I wanted to put this out for discussion/review to get the UI and protocol sorted out; a better implementation of |
This is a revised version of #949.
Changes from #949 include:
--eaw-wide
switch.Notes:
I did a bunch of testing with various formats to see what was most space efficient. Text proved more efficient than protobufs, lists of run lengths were better than lists of offsets. The 9->11 overlay is 946 bytes uncompressed and 350 bytes gzipped standalone (actual compression will be something slightly different in Mosh packets).
Changing the character width tables is a change in the state of the terminal emulator. I see two ways to handle that: either set the tables before the user session is allowed to start, or fully integrate the character width tables into the terminal state machine. I chose to set the tables at startup. I think to fully integrate the state, we would have to send the overlay table to the server, which would then have to send it (or references to the changes) back to the client to keep it in sync. Messy.
With a small overlay, nothing extra needs to be done to Mosh to hold back the user session-- a while ago, I added some functionality on the server to wait until the first message is received. But if the overlay ever gets so large that we decide we need to send it in pieces in multiple messages, then we need to add some functionality for that. That's what the hold-session commits are for. I lean towards including this as currently-unused functionality so it's available for future versions of Mosh, but I'm not sure about that yet.