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

WIP: Fix Format-Table/Format-Wide padding when output contains CJK characters #5739

Closed
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@kwkam
Contributor

kwkam commented Dec 22, 2017

PR Summary

Compute display width for the string in LengthInBufferCells, otherwise Format-Table and Format-Wide will pad unnecessary space for CJK character.

Add UTF-8 to checking list since it contains CJK characters.

PR Checklist

Note: Please mark anything not applicable to this PR NA.

console: fix cjk char render on tab completion
Solve an issue where CJK characters are not being rendered with
correct width while tab completion is cycling multiple results,
caused by reusing a buffer without resetting lead/trail attribute.
Also add UTF-8 to checking list since it contains CJK characters.

@kwkam kwkam requested review from anmenaga, charub, daxian-dbw and lzybkr as code owners Dec 22, 2017

@iSazonov

This comment has been minimized.

Collaborator

iSazonov commented Dec 22, 2017

@kwkam Thanks for your contribution! Please follow the requirements of PR Checklist.

@lzybkr

This comment has been minimized.

Member

lzybkr commented Dec 22, 2017

@kwkam - This might be fixed in the PSReadLine repo. You can install v2 from the gallery.

I think the version in this repo will be removed in favor of pulling my version from the gallery, so if you still see issues, I suggest you submit a PR there instead.

The change to ConsoleControl.cs does make sense in this repo though, so we should merge that for sure.

@kwkam kwkam changed the title from console: fix cjk char render on tab completion to Console: Fix CJK char render when using UTF-8 output encoding Dec 23, 2017

@kwkam

This comment has been minimized.

Contributor

kwkam commented Dec 23, 2017

@lzybkr - Sorry I didn't know PSReadLine is an external module and yes the issue is fixed in upstream.

kwkam added some commits Dec 27, 2017

console: fix format-table/wide in UTF-8 when contains CJK char
LengthInBufferCells should compute the real string length
@@ -2781,8 +2781,17 @@ internal static int LengthInBufferCells(string str, int offset, bool checkEscape
escapeSequenceAdjustment += ControlSequenceLength(str, ref i);
}
}
#if !UNIX

This comment has been minimized.

@lzybkr

lzybkr Dec 27, 2017

Member

Isn't this a problem on Linux/Mac as well?

This comment has been minimized.

@kwkam

kwkam Jan 9, 2018

Contributor

I suppose but I cannot test.
Is there any reason why the LengthInBufferCells function is disabled on unix?

This comment has been minimized.

@lzybkr

lzybkr Jan 9, 2018

Member

I don't know for sure, but I would guess the original code depended on Windows apis and when porting to non-Windows, they punted on porting that code until a later time.

This comment has been minimized.

@kwkam

kwkam Jan 10, 2018

Contributor

Exlcuding LengthInBufferCells from that block since it seems to be harmless.

@kwkam kwkam changed the title from Console: Fix CJK char render when using UTF-8 output encoding to Fix Format-Table/Format-Wide padding when output contains CJK characters Mar 16, 2018

kwkam added some commits Mar 16, 2018

@kwkam kwkam changed the title from Fix Format-Table/Format-Wide padding when output contains CJK characters to WIP: Fix Format-Table/Format-Wide padding when output contains CJK characters Mar 16, 2018

@stale

This comment has been minimized.

stale bot commented May 12, 2018

This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days.
Thank you for your contributions.
Community members are welcome to grab these works.

@stale stale bot added the Review - Abandoned label May 12, 2018

@stale

This comment has been minimized.

stale bot commented May 22, 2018

This PR has be automatically close because it is stale. If you wish to continue working on the PR, please first update the PR, then reopen it.
Thanks again for your contribution.
Community members are welcome to grab these works.

@stale stale bot closed this May 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment