On Fri, 7 Apr 2023 19:01:34 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:

>> The TestScaffold incorrectly parse options, it should insert wrapper class 
>> between VM options and applications classame.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   updated parsing.

A small nit but otherwise the new approach to parsing args now seems 
complete/consistent.

Thanks.

test/jdk/com/sun/jdi/TestScaffold.java line 478:

> 476:         // The result with wrapper enabled:
> 477:         // argInfo.targetAppCommandLine : TestScaffold Virtual 
> Frames2Targ
> 478:         // argInfo.targetVMArgs : -Xss4M --enable-preview

This example may need updating now virtual threads are no longer in preview 
mode.

test/jdk/com/sun/jdi/TestScaffold.java line 501:

> 499:             } else if (arg.startsWith("-J")) {
> 500:                 argInfo.targetVMArgs += (arg.substring(2) + ' ');
> 501:                 throw new RuntimeException("-J-option is not supported. 
> Incorrect arg: " + args[i]);

Shouldn't line 500 be deleted now and the exception just print `arg`?

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

PR Review: https://git.openjdk.org/jdk/pull/13170#pullrequestreview-1378508428
PR Review Comment: https://git.openjdk.org/jdk/pull/13170#discussion_r1162352554
PR Review Comment: https://git.openjdk.org/jdk/pull/13170#discussion_r1162341404

Reply via email to