On Thu, 5 Dec 2024 17:09:47 GMT, Henry Jen <henry...@openjdk.org> wrote:
>> Improving option value handling to support passing argument value starts
>> with "--".
>>
>> Before the fix, in following example, --add-modules will be considered as
>> another option for JLink instead of argument value for --add-options.
>> --add-options --add-modules=jdk.incubator.concurrent
>> --add-options=--add-modules=jdk.incubator.concurrent
>>
>> will cause JLink to report
>> Error: no value given for --add-options
>> as --add-modules is considered another option for JLink.
>>
>> After the fix, by using = will ensure the value is properly handled as
>> argument value. Also using "" with multiple values will be recognized
>> properly. So following form should work
>> --add-options "--add-modules jdk.incubator.concurrent"
>> --add-options=--add-modules=jdk.incubator.concurrent
>
> Henry Jen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix style
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java line 552:
> 550: if (potentiallyGnuOption && param.length() >= 2
> &&
> 551: param.charAt(0) == '-' && param.charAt(1) ==
> '-' &&
> 552: !param.contains(" ")) {
Why it does not accept this case `--add-options
--add-modules=jdk.incubator.concurrent`?
test/jdk/tools/jlink/TaskHelperTest.java line 2:
> 1: /*
> 2: * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights
> reserved.
Suggestion:
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
test/jdk/tools/jlink/TaskHelperTest.java line 115:
> 113: { "--raw-arg-plugin", "--main-no-arg --list",
> "--main-no-arg"},
> 114: { "--raw-arg-plugin", " --main-no-arg", "--main-no-arg" },
> 115: };
Skimming on the test, I don't see validation of the parsed parameter values. I
expect the test should also verify.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22526#discussion_r1868458765
PR Review Comment: https://git.openjdk.org/jdk/pull/22526#discussion_r1868442074
PR Review Comment: https://git.openjdk.org/jdk/pull/22526#discussion_r1868460382
