On Tue, 3 Dec 2024 22:43:17 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> 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`?
--style argument still could be a real jlink argument, better check would be to
see if that's a valid jlink option rather than just "--".
The other alternative would be like what proposed by @YaSuenag, simply take the
next token as value.
> 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.
Line 126 validate the argument is processed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22526#discussion_r1871806616
PR Review Comment: https://git.openjdk.org/jdk/pull/22526#discussion_r1871807961
