On Fri, 15 Nov 2024 14:20:00 GMT, Alexey Semenyuk <asemen...@openjdk.org> wrote:

>> Taizo Kurashige has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix indent
>
> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java line 344:
> 
>> 342:             command.add("cmd.exe");
>> 343:             command.add("/c");
>> 344:             command.add("chcp 65001 && " + 
>> printCommandLine(executablePath().toString(), args));
> 
> `printCommandLine()` function does a lousy job of escaping whitespaces in 
> arguments. Its output is for logging purposes only. 
> 
> I experimented with the use of "chcp" in the "Executor" class and figured 
> that it is not necessary to put the entire command line in a single argument. 
> Please take a look at 
> https://github.com/openjdk/jdk/commit/7975a8e97485b999691b986a78b1e0b47a9b30a1.
>  It suppresses the output of "chcp" command, so client code may remain 
> unchanged. Need to add the missing `setWinEnableUTF8()` and good to go.

Sorry for the incorrect use of `printCommandLine()`.

As I checked, I could certainly execute "chcp" without putting the entire 
command line in a single argument. I fixed referring to 
https://github.com/openjdk/jdk/commit/7975a8e97485b999691b986a78b1e0b47a9b30a1. 
Thanks.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22142#discussion_r1849482866

Reply via email to