[go] os: Use GetConsoleCP() instead of GetACP()

[Yasuhiro MATSUMOTO (Gerrit)]

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: Use GetConsoleCP() instead of GetACP()

Console CodePage not always be same as Active CodePage.

Fixes #16857

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go
3 files changed, 11 insertions(+), 3 deletions(-)


--
https://go-review.googlesource.com/27575

[Yasuhiro MATSUMOTO (Gerrit)]

Yasuhiro MATSUMOTO has posted comments on this change.

os: Use GetConsoleCP() instead of GetACP()

Patch Set 2:

(1 comment)

https://go-review.googlesource.com/#/c/27575/2/src/internal/syscall/windows/syscall_windows.go
File src/internal/syscall/windows/syscall_windows.go:

Line 140: //sys GetACP() (acp uint32) = kernel32.GetACP
Alex: GetACP is in internal. So can we remove this?


--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>
Gerrit-HasComments: Yes

[Alex Brainman (Gerrit)]

Alex Brainman has posted comments on this change.

os: Use GetConsoleCP() instead of GetACP()

Patch Set 2:

(2 comments)

https://go-review.googlesource.com/#/c/27575/2//COMMIT_MSG
Commit Message:

Line 7: os: Use GetConsoleCP() instead of GetACP()
s/Use/use/


Line 9: Console CodePage not always be same as Active CodePage.
Why didn't you use GetConsoleCP (instead of GetACP) before? What is the
difference between these functions? Why GetConsoleCP and GetACP return
different values on computer mentioned on issue 16857?

I think you should put some of these answers here in CL description.

[Alex Brainman (Gerrit)]

Alex Brainman has posted comments on this change.

os: Use GetConsoleCP() instead of GetACP()

Patch Set 2:

(1 comment)

https://go-review.googlesource.com/#/c/27575/2/src/internal/syscall/windows/syscall_windows.go
File src/internal/syscall/windows/syscall_windows.go:

Line 140: //sys GetACP() (acp uint32) = kernel32.GetACP
> Alex: GetACP is in internal. So can we remove this?

I wouldn't worry about that. Lets leave it alone for the moment.

[Yasuhiro MATSUMOTO (Gerrit)]

Reviewers: Alex Brainman

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: use GetConsoleCP() instead of GetACP()

Console CodePage not always be same as Active CodePage. For example, in
Windows uses charset 1252 as default for west-european countries but it
uses usually 850 for the console for west-european countries.

Fixes #16857

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go
3 files changed, 11 insertions(+), 3 deletions(-)

[Yasuhiro MATSUMOTO (Gerrit)]

Yasuhiro MATSUMOTO has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 2:

(3 comments)

https://go-review.googlesource.com/#/c/27575/2//COMMIT_MSG
Commit Message:

Line 7: os: Use GetConsoleCP() instead of GetACP()
> s/Use/use/
Done

Line 9: Console CodePage not always be same as Active CodePage.
> Why didn't you use GetConsoleCP (instead of GetACP) before? What is the
> dif

I didn't know that there are environment that return different values
between GetConsoleCP and GetACP. So I believe GetACP() is enough to do this.

https://go-review.googlesource.com/#/c/27575/2/src/internal/syscall/windows/syscall_windows.go
File src/internal/syscall/windows/syscall_windows.go:

Line 140: //sys GetACP() (acp uint32) = kernel32.GetACP

> I wouldn't worry about that. Lets leave it alone for the moment.

Done

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: Yes

[Yasuhiro MATSUMOTO (Gerrit)]

Reviewers: Alex Brainman

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: use GetConsoleCP() instead of GetACP()

Console CodePage not always be same as Active CodePage. For example, in
Windows uses charset 1252 as default for west-european countries but it
uses usually 850 for the console for west-european countries.

Fixes #16857

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go

3 files changed, 16 insertions(+), 3 deletions(-)

[Yasuhiro MATSUMOTO (Gerrit)]

Reviewers: Alex Brainman

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: use GetConsoleCP() instead of GetACP()

Console CodePage not always be same as Active CodePage. For example, in
Windows uses charset 1252 as default for west-european countries but it
uses usually 850 for the console for west-european countries.

CP1521 should be decomposed with MB_PRECOMPOSED.

[Hiroshi Ioka (Gerrit)]

Hiroshi Ioka has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 5:

(2 comments)

https://go-review.googlesource.com/#/c/27575/5//COMMIT_MSG
Commit Message:

Line 12: CP1521 should be decomposed with MB_PRECOMPOSED.
CP1521? Probably, CP1252.
"decomposed with MB_PRECOMPOSED" is non-sense.
you are trying to use precomposed characters instead of decomposed
characters.
and I don't see any reason for 'should be' here.
I think using precomposed characters is reasonable.
because they are commonly used except HFS+.
but it's not codepage specific.


https://go-review.googlesource.com/#/c/27575/5/src/os/file_windows.go
File src/os/file_windows.go:

Line 226: nwc, err := windows.MultiByteToWideChar(ccp,
windows.MB_PRECOMPOSED, pmb, int32(nmb), nil, 0)
https://msdn.microsoft.com/en-us/library/windows/desktop/dd319072(v=vs.85).aspx
says:

For the code pages listed below, dwFlags must be set to 0. Otherwise, the
function fails with ERROR_INVALID_FLAGS.

So,

flags := windows.MB_PRECOMPOSED
switch ccp {
case 50220, 50221, ...:
flags = 0

}


--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>

Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>

[Yasuhiro MATSUMOTO (Gerrit)]

Reviewers: Alex Brainman, Hiroshi Ioka

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: use GetConsoleCP() instead of GetACP()

Console CodePage not always be same as Active CodePage. For example, in
Windows uses charset 1252 as default for west-european countries but it
uses usually 850 for the console for west-european countries.

For CP1251, MB_PRECOMPOSED must be used.

Fixes #16857

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go

3 files changed, 40 insertions(+), 3 deletions(-)

[Yasuhiro MATSUMOTO (Gerrit)]

Yasuhiro MATSUMOTO has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 5:

(2 comments)

Thanks for the review. The code pages listed bits many, So created map for
checking this.

https://go-review.googlesource.com/#/c/27575/5//COMMIT_MSG
Commit Message:

Line 12: CP1521 should be decomposed with MB_PRECOMPOSED.
> CP1521? Probably, CP1252.

Done

https://go-review.googlesource.com/#/c/27575/5/src/os/file_windows.go
File src/os/file_windows.go:

Line 226: nwc, err := windows.MultiByteToWideChar(ccp,
windows.MB_PRECOMPOSED, pmb, int32(nmb), nil, 0)
> https://msdn.microsoft.com/en-us/library/windows/desktop/dd319072(v=vs.85).

Done

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: Yes

[Hiroshi Ioka (Gerrit)]

Hiroshi Ioka has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 6:

(3 comments)

https://go-review.googlesource.com/#/c/27575/6//COMMIT_MSG
Commit Message:

Line 12: For CP1251, MB_PRECOMPOSED must be used.
CP1252. Why 'must be'? can you write a reason for that?


https://go-review.googlesource.com/#/c/27575/6/src/os/file_windows.go
File src/os/file_windows.go:

Line 18: var composingCodePages map[uint32]bool
What does composingCodePages mean?
How about cpFlagsDisabled map[uint32]bool?


Line 22: // See
https://msdn.microsoft.com/ja-jp/library/windows/desktop/dd319072(v=vs.85).aspx
I think en-us page is better.

[Hiroshi Ioka (Gerrit)]

Hiroshi Ioka has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 6:

(1 comment)

https://go-review.googlesource.com/#/c/27575/6/src/os/file_windows.go
File src/os/file_windows.go:

Line 34: composingCodePages[42] = true
> Note For UTF-8 or code page 54936 (GB18030, starting with Windows
> Vista), dwFlags must be set to either 0 or MB_ERR_INVALID_CHARS.

> Otherwise, the function fails with ERROR_INVALID_FLAGS.

So, 54936 also?

[Giovanni Bajo (Gerrit)]

Giovanni Bajo has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 6:

(2 comments)

https://go-review.googlesource.com/#/c/27575/6//COMMIT_MSG
Commit Message:

PS6, Line 9: Active
ACP means ANSI CodePage, not Active CodePage


https://go-review.googlesource.com/#/c/27575/6/src/os/file_windows.go
File src/os/file_windows.go:

PS6, Line 246: flag := uint32(windows.MB_PRECOMPOSED)
: if _, ok := composingCodePages[ccp]; ok {
: flag = 0
: }
MB_PRECOMPOSED is the default, so it should be the same as passing 0. Why
do you need this code?

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>

Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>

[Yasuhiro MATSUMOTO (Gerrit)]

Yasuhiro MATSUMOTO has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 6:

(1 comment)

https://go-review.googlesource.com/#/c/27575/6/src/os/file_windows.go
File src/os/file_windows.go:

PS6, Line 246: flag := uint32(windows.MB_PRECOMPOSED)
: if _, ok := composingCodePages[ccp]; ok {
: flag = 0
: }
> MB_PRECOMPOSED is the default, so it should be the same as passing 0. Why
> d

okay, let's specify 0 for the second argument.

@hiroshiioka how about this?

[Hiroshi Ioka (Gerrit)]

Hiroshi Ioka has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 6:

(1 comment)

https://go-review.googlesource.com/#/c/27575/6/src/os/file_windows.go
File src/os/file_windows.go:

PS6, Line 246: flag := uint32(windows.MB_PRECOMPOSED)
: if _, ok := composingCodePages[ccp]; ok {
: flag = 0
: }

> okay, let's specify 0 for the second argument.

SGTM. A little concern is msdn doesn't say that default flag is zero,
though.

[Hiroshi Ioka (Gerrit)]

Hiroshi Ioka has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 6:

(1 comment)

https://go-review.googlesource.com/#/c/27575/6/src/os/file_windows.go
File src/os/file_windows.go:

PS6, Line 246: flag := uint32(windows.MB_PRECOMPOSED)
: if _, ok := composingCodePages[ccp]; ok {
: flag = 0
: }

> SGTM. A little concern is msdn doesn't say that default flag is zero,
> thoug

I think I was wrong. just ignore my concern.

[Yasuhiro MATSUMOTO (Gerrit)]

Reviewers: Giovanni Bajo, Alex Brainman, Hiroshi Ioka

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: use GetConsoleCP() instead of GetACP()

Console CodePage not always be same as ANSI CodePage. For example, in

Windows uses charset 1252 as default for west-european countries but it
uses usually 850 for the console for west-european countries.

Use MB_PRECOMPOSED for MultiByteToWideChar.

Fixes #16857

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go

3 files changed, 17 insertions(+), 3 deletions(-)

[Yasuhiro MATSUMOTO (Gerrit)]

Yasuhiro MATSUMOTO has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 6:

(8 comments)

Thanks all.

https://go-review.googlesource.com/#/c/27575/6//COMMIT_MSG
Commit Message:

PS6, Line 9: Active
> ACP means ANSI CodePage, not Active CodePage

Done

Line 12: For CP1251, MB_PRECOMPOSED must be used.
> CP1252. Why 'must be'? can you write a reason for that?

Done


https://go-review.googlesource.com/#/c/27575/6/src/os/file_windows.go
File src/os/file_windows.go:

Line 18: var composingCodePages map[uint32]bool
> What does composingCodePages mean?

Done

Line 22: // See
https://msdn.microsoft.com/ja-jp/library/windows/desktop/dd319072(v=vs.85).aspx
> I think en-us page is better.

Done

Line 34: composingCodePages[42] = true
> >Note For UTF-8 or code page 54936 (GB18030, starting with Windows
> Vista),

Done

PS6, Line 246: flag := uint32(windows.MB_PRECOMPOSED)
: if _, ok := composingCodePages[ccp]; ok {
: flag = 0
: }
> SGTM. A little concern is msdn doesn't say that default flag is zero,
> thoug

Done

PS6, Line 246: flag := uint32(windows.MB_PRECOMPOSED)
: if _, ok := composingCodePages[ccp]; ok {
: flag = 0
: }

> MB_PRECOMPOSED is the default, so it should be the same as passing 0. Why
> d

Done

PS6, Line 246: flag := uint32(windows.MB_PRECOMPOSED)
: if _, ok := composingCodePages[ccp]; ok {
: flag = 0
: }

> I think I was wrong. just ignore my concern.

Done

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: Yes

[Hiroshi Ioka (Gerrit)]

Hiroshi Ioka has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 7:

(1 comment)

https://go-review.googlesource.com/#/c/27575/7//COMMIT_MSG
Commit Message:

Line 12: Use MB_PRECOMPOSED for MultiByteToWideChar.
Confusing. You are not using MB_PRECOMPOSED, but using 0, aren't you? And
you don't explain a reason for this change, yet.

[Yasuhiro MATSUMOTO (Gerrit)]

Reviewers: Giovanni Bajo, Alex Brainman, Hiroshi Ioka

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: use GetConsoleCP() instead of GetACP()

Console CodePage not always be same as ANSI CodePage. For example, in
Windows uses charset 1252 as default for west-european countries but it
uses usually 850 for the console for west-european countries.

Use default value zero for the dwFlags of MultiByteToWideChar. It's same
as MF_PRECOMPOSED. Go handle single Unicode code point if defined it for
a character.

Fixes #16857

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go

3 files changed, 12 insertions(+), 3 deletions(-)

[Hiroshi Ioka (Gerrit)]

Hiroshi Ioka has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 8:

(3 comments)

https://go-review.googlesource.com/#/c/27575/8//COMMIT_MSG
Commit Message:

Line 11: uses usually 850 for the console for west-european countries.
append empty line


Line 13: as MF_PRECOMPOSED. Go handle single Unicode code point if defined
it for
I'm sorry. I still cannot see good reason. What problem is there? How does
this change solve it? You've already explained reason for
s/GetACP/GetConsoleCP/g. But you still don't explain this.


https://go-review.googlesource.com/#/c/27575/8/src/os/file_windows.go
File src/os/file_windows.go:

Line 226: // Specify MB_PRECOMPOSED for the dwFlags
It's not true.

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: Yes

[Yasuhiro MATSUMOTO (Gerrit)]

Reviewers: Giovanni Bajo, Alex Brainman, Hiroshi Ioka

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: use GetConsoleCP() instead of GetACP()

Console CodePage not always be same as ANSI CodePage. For example, in
Windows uses charset 1252 as default for west-european countries but it

uses usually 850 for the console for west-european countries.

Use default value zero for the dwFlags of MultiByteToWideChar. It's same

as MF_PRECOMPOSED. Go is designed to handle single Unicode code point if
defined it for a character. But MF_COMPOSED was used for this part.
MF_COMPOSED mean that a character is represented by combining characters.
Ex: umlaut is represented by capital A and combining diaeresis. This
change fix to convert input bytes from console to pre-composed utf-8
bytes (to return bytes of umlaut in utf-8 it self).

Fixes #16857

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go
3 files changed, 12 insertions(+), 3 deletions(-)

[Yasuhiro MATSUMOTO (Gerrit)]

Yasuhiro MATSUMOTO has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 8:

(3 comments)

https://go-review.googlesource.com/#/c/27575/7//COMMIT_MSG
Commit Message:

Line 12: Use default value zero for the dwFlags of MultiByteToWideChar.
It's same

> Confusing. You are not using MB_PRECOMPOSED, but using 0, aren't you? And
> y

Fixed. Could you please check last change.


https://go-review.googlesource.com/#/c/27575/8//COMMIT_MSG
Commit Message:

Line 11: uses usually 850 for the console for west-european countries.
> append empty line
Done


Line 13: as MF_PRECOMPOSED. Go handle single Unicode code point if defined
it for

> I'm sorry. I still cannot see good reason. What problem is there? How does

Done

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: Yes

[Yasuhiro MATSUMOTO (Gerrit)]

Reviewers: Tobias Kunicke, Giovanni Bajo, Alex Brainman, Hiroshi Ioka

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: use GetConsoleCP() instead of GetACP()

Console CodePage not always be same as ANSI CodePage. For example, in
Windows uses charset 1252 as default for west-european countries but it

uses usually 850 for the console for west-european countries.

Use default value zero for the dwFlags of MultiByteToWideChar. It's same

as MB_PRECOMPOSED. Go is designed to handle single Unicode code point if
defined it for a character. But MB_COMPOSITE was used for this part.
MB_COMPOSITE mean that a character is represented by combining characters.

Ex: umlaut is represented by capital A and combining diaeresis. This
change fix to convert input bytes from console to pre-composed utf-8
bytes (to return bytes of umlaut in utf-8 it self).

Fixes #16857

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go
3 files changed, 12 insertions(+), 3 deletions(-)

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>

Gerrit-Reviewer: Tobias Kunicke <tob...@kunicke.eu>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

[Yasuhiro MATSUMOTO (Gerrit)]

Yasuhiro MATSUMOTO has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 9:

(3 comments)

Thank you!

https://go-review.googlesource.com/#/c/27575/9//COMMIT_MSG
Commit Message:

Line 14: as MF_PRECOMPOSED. Go is designed to handle single Unicode code
point if
> MF_PRECOMPOSED -> MB_PRECOMPOSED
Done


Line 15: defined it for a character. But MF_COMPOSED was used for this part.
> MF_COMPOSED -> MB_COMPOSITE
Done


Line 16: MF_COMPOSED mean that a character is represented by combining
characters.
> MF_COMPOSED -> MB_COMPOSITE
Done

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>
Gerrit-Reviewer: Tobias Kunicke <tob...@kunicke.eu>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: Yes

[Giovanni Bajo (Gerrit)]

Giovanni Bajo has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 10: Code-Review+1

(2 comments)

https://go-review.googlesource.com/#/c/27575/9//COMMIT_MSG
Commit Message:

Line 23: Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
I think this commit message is too large and doesn't address the correct
points. Consider that this whole issue of MB_PRECOMPOSED is a little moot;
we are using the default, which is the Unicode NFC form, which is the same
defaults for almost all Unicode-handling library and systems in the world
(with a very few exceptions), so it's not something that should be
highlighted in a commit message. I suggest this phrasing:

os: use GetConsoleCP() instead of GetACP()

It is possible (and common) for Windows systems to use a different codepage
for console applications from that used on normal windowed application
(called ANSI codepage); for instance, most of the western Europe uses CP850
for console (for backward compatibility with MS-DOS), while windowed
applications use a different codepage depending on the country (eg: CP1252
aka Latin-1). The usage being changed with this commit is specifically
related to decoding input coming from the console, so the previous usage of
the ANSI codepage was wrong.

Fixes #16857.


https://go-review.googlesource.com/#/c/27575/9/src/os/file_windows.go
File src/os/file_windows.go:

PS9, Line 226: // Specify MB_PRECOMPOSED for the dwFlags
This comment is confusing because it says that we specify a flag while the
the code doesn't use it.

If you want to leave a trace of the issue that was discussed here, I would
write something like:

// Convert from 8-bit console encoding to UTF16.
// MultiByteToWideChar defaults to Unicode NFC form, which is the expected
one.

[Yasuhiro MATSUMOTO (Gerrit)]

Reviewers: Tobias Kunicke, Giovanni Bajo, Alex Brainman, Hiroshi Ioka

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: use GetConsoleCP() instead of GetACP()

It is possible (and common) for Windows systems to use a different codepage
for console applications from that used on normal windowed application
(called ANSI codepage); for instance, most of the western Europe uses
CP850 for console (for backward compatibility with MS-DOS), while
windowed applications use a different codepage depending on the country
(eg: CP1252 aka Latin-1). The usage being changed with this commit is
specifically related to decoding input coming from the console, so the
previous usage of the ANSI codepage was wrong.

Fixes #16857.

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go

3 files changed, 13 insertions(+), 3 deletions(-)

[Yasuhiro MATSUMOTO (Gerrit)]

Yasuhiro MATSUMOTO has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 9:

(2 comments)

Thank you so much.

https://go-review.googlesource.com/#/c/27575/9//COMMIT_MSG
Commit Message:

Line 23: Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
> I think this commit message is too large and doesn't address the correct
> po

Done

https://go-review.googlesource.com/#/c/27575/9/src/os/file_windows.go
File src/os/file_windows.go:

PS9, Line 226: // Specify MB_PRECOMPOSED for the dwFlags
> This comment is confusing because it says that we specify a flag while the

Done

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>
Gerrit-Reviewer: Tobias Kunicke <tob...@kunicke.eu>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: Yes

[Hiroshi Ioka (Gerrit)]

Hiroshi Ioka has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 11:

I'm tired to say same things 4 times.
This CL contains two changes.

A: GetConsoleCP -> GetACP
B: NFD -> NFC

Obviously two changes are not related each other.
While you've already explained A, you've never explained B.
Do you think implicit behavior change is good?

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>
Gerrit-Reviewer: Tobias Kunicke <tob...@kunicke.eu>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: No

[Hiroshi Ioka (Gerrit)]

Hiroshi Ioka has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 11:

A: GetACP -> GetConsoleCP

[Yasuhiro MATSUMOTO (Gerrit)]

Reviewers: Tobias Kunicke, Giovanni Bajo, Alex Brainman, Hiroshi Ioka

Yasuhiro MATSUMOTO uploaded a new patch set:
https://go-review.googlesource.com/27575

os: use GetConsoleCP() instead of GetACP()

It is possible (and common) for Windows systems to use a different codepage
for console applications from that used on normal windowed application
(called ANSI codepage); for instance, most of the western Europe uses
CP850 for console (for backward compatibility with MS-DOS), while
windowed applications use a different codepage depending on the country
(eg: CP1252 aka Latin-1). The usage being changed with this commit is
specifically related to decoding input coming from the console, so the
previous usage of the ANSI codepage was wrong.

Also fixes an issue that previous did convert bytes as NFD. Go is
designed to handle single Unicode code point. This fix change behaivor
to NFC.

Fixes #16857.

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go
3 files changed, 13 insertions(+), 3 deletions(-)

[Yasuhiro MATSUMOTO (Gerrit)]

Yasuhiro MATSUMOTO has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 12:

> A: GetACP -> GetConsoleCP

Sorry & Thanks in many times.

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>
Gerrit-Reviewer: Tobias Kunicke <tob...@kunicke.eu>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: No

[Hiroshi Ioka (Gerrit)]

Hiroshi Ioka has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 12: Code-Review+1

(1 comment)

https://go-review.googlesource.com/#/c/27575/12//COMMIT_MSG
Commit Message:

Line 19: designed to handle single Unicode code point. This fix change
behaivor
I'm not sure "Go is designed to handle single Unicode code point", and
maybe there are grammar issues, but otherwise LGTM.

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>
Gerrit-Reviewer: Tobias Kunicke <tob...@kunicke.eu>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: Yes

[Tobias Kunicke (Gerrit)]

Tobias Kunicke has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 9:

(3 comments)

https://go-review.googlesource.com/#/c/27575/9//COMMIT_MSG
Commit Message:

Line 14: as MF_PRECOMPOSED. Go is designed to handle single Unicode code
point if
MF_PRECOMPOSED -> MB_PRECOMPOSED

Line 15: defined it for a character. But MF_COMPOSED was used for this part.
MF_COMPOSED -> MB_COMPOSITE

Line 16: MF_COMPOSED mean that a character is represented by combining
characters.
MF_COMPOSED -> MB_COMPOSITE

[Alex Brainman (Gerrit)]

Alex Brainman has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 12: Run-TryBot+1 Code-Review+2

I was investigating issue 17097. And I changed my mind about this CL. It
LGTM. I will submit it tomorrow unless others object.

Thank you.

Alex

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>
Gerrit-Reviewer: Hiroshi Ioka <hiroch...@gmail.com>
Gerrit-Reviewer: Tobias Kunicke <tob...@kunicke.eu>
Gerrit-Reviewer: Yasuhiro MATSUMOTO <matt...@gmail.com>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 12:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=c605bf26

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 12:

Build is still in progress...
This change failed on darwin-amd64-10_11:
See
https://storage.googleapis.com/go-build-log/c605bf26/darwin-amd64-10_11_85894227.log

Consult https://build.golang.org/ to see whether it's a new failure. Other
builds still in progress; subsequent failure notices suppressed until final
report.

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 12: TryBot-Result-1

1 of 16 TryBots failed:
Failed on darwin-amd64-10_11:
https://storage.googleapis.com/go-build-log/c605bf26/darwin-amd64-10_11_85894227.log

Consult https://build.golang.org/ to see whether they are new failures.

--
https://go-review.googlesource.com/27575
Gerrit-Reviewer: Alex Brainman <alex.b...@gmail.com>
Gerrit-Reviewer: Giovanni Bajo <ra...@develer.com>

Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

[Alex Brainman (Gerrit)]

Alex Brainman has posted comments on this change.

os: use GetConsoleCP() instead of GetACP()

Patch Set 12: -Run-TryBot

> 1 of 16 TryBots failed:
> Failed on darwin-amd64-10_11:
https://storage.googleapis.com/go-build-log/c605bf26/darwin-amd64-10_11_85894227.log
>

I don't see how the failure is related to CL changes.

Alex

[Alex Brainman (Gerrit)]

Alex Brainman has submitted this change and it was merged.

os: use GetConsoleCP() instead of GetACP()

It is possible (and common) for Windows systems to use a different codepage
for console applications from that used on normal windowed application
(called ANSI codepage); for instance, most of the western Europe uses
CP850 for console (for backward compatibility with MS-DOS), while
windowed applications use a different codepage depending on the country
(eg: CP1252 aka Latin-1). The usage being changed with this commit is
specifically related to decoding input coming from the console, so the
previous usage of the ANSI codepage was wrong.

Also fixes an issue that previous did convert bytes as NFD. Go is

designed to handle single Unicode code point. This fix change behaivor

to NFC.

Fixes #16857.

Change-Id: I4f41ae83ece47321b6e9a79a2087ecbb8ac066dd

Reviewed-on: https://go-review.googlesource.com/27575
Reviewed-by: Hiroshi Ioka <hiroch...@gmail.com>
Reviewed-by: Alex Brainman <alex.b...@gmail.com>

---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/file_windows.go
3 files changed, 13 insertions(+), 3 deletions(-)

Approvals:
Alex Brainman: Looks good to me, approved
Hiroshi Ioka: Looks good to me, but someone else must approve

Objections:
Gobot Gobot: TryBots failed