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 dynamic changing of font in Windows console on CJK codepages #771

Merged
merged 4 commits into from Oct 10, 2018

Conversation

Projects
None yet
2 participants
@SteveL-MSFT
Collaborator

SteveL-MSFT commented Oct 2, 2018

On Windows, the outputencoding is tied to a codepage which is tied to a font. When using a CJK locale, the default is using a raster font that can render the CJK glyphs. Previously, PSReadLine 2.0.0 was manipulating the OutputEncoding to utf8 which resulted in a visible font change in the console and the resulting rendering in the screen buffer was not correct.

The fix is to detect that a raster font is being used and not change the output encoding. Also in cases where the output encoding is already utf8, there's no need to change it.

@SteveL-MSFT SteveL-MSFT requested a review from lzybkr Oct 2, 2018

@lzybkr

This comment has been minimized.

Owner

lzybkr commented Oct 3, 2018

If PowerShell is not running in a console, this code probably won't work as expected - for example if we are running under ConPTY, so we should avoid checking the font in that case.

I think you can check this easily enough by calling GetFileType on the file handle from STD_INPUT_HANDLE. If the return value is not FILE_TYPE_CHAR, then we can skip calling checking the font.

@SteveL-MSFT SteveL-MSFT force-pushed the SteveL-MSFT:outputencoding-rasterfont branch from 2f4e5fa to f6906f7 Oct 4, 2018

@SteveL-MSFT SteveL-MSFT force-pushed the SteveL-MSFT:outputencoding-rasterfont branch from f6906f7 to 668b01d Oct 4, 2018

private static bool IsHandleRedirected(bool stdin)
{
var handle = GetStdHandle((uint)(stdin ? StandardHandleId.Input : StandardHandleId.Output));
// If handle is not to a character device, we must be redirected:
int fileType = GetFileType(handle);
if ((fileType & FILE_TYPE_CHAR) != FILE_TYPE_CHAR)

This comment has been minimized.

@SteveL-MSFT

SteveL-MSFT Oct 4, 2018

Collaborator

Per the documentation, GetFileType() returns a single value and is not flags.

Show resolved Hide resolved PSReadLine/ReadLine.cs Outdated
@lzybkr

lzybkr approved these changes Oct 10, 2018

@lzybkr lzybkr merged commit 1d8dd4a into lzybkr:master Oct 10, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@SteveL-MSFT SteveL-MSFT deleted the SteveL-MSFT:outputencoding-rasterfont branch Oct 10, 2018

lzybkr added a commit that referenced this pull request Dec 3, 2018

Fix Raster font check (#825)
The original fix in #771 resulted in essentially never setting the UTF8
output encoding because it only tested the fixed width bit on the font
family, but every console font is fixed width.

The proper fix appears to be testing that all 4 lower bits in FontFamily
are not set.

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