On Tue, 25 Jun 2024 13:55:32 GMT, Sonia Zaldana Calles <szald...@openjdk.org> 
wrote:

>> Hi all, 
>> 
>> This PR addresses [8332124](https://bugs.openjdk.org/browse/JDK-8332124) 
>> enabling jcmd to accept "help" as an argument to subcommands. 
>> 
>> Testing: 
>> - [x] Verified running `jcmd 4711 VM.metaspace help` works along with other 
>> subcommands. 
>> - [x] Added test case passes. 
>> 
>> Thanks, 
>> Sonia
>
> Sonia Zaldana Calles has updated the pull request with a new target base due 
> to a merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8332124
>  - Adding test case for suboptions with trailing spaces and adding null 
> terminator to reordered command
>  - 8332124: Jcmd processing should accept the "help" sub option as command 
> argument

I could benefit from this, I think it's natural to sometimes want to check the 
help after you've started typing, so I like the idea.  But can it actually 
clash with any other usage?

e.g. "help" can be a filename.  We are making the filename "help" unusable.  
Maybe not a big deal to most people, but we do need to make this clear.


jcmd PID Thread.dump_to_file help

...used to create a file "help", and now does not?

Compiler.directives_add takes a filename and would accept "help".
GC.heap_dump used to create a dump file called "help".

jcmd 7537 VM.set_flag help
..could have been a clash if we had such a VM flag (not likely...).


I don't mean to labour artificial or uncommon cases, but we do want this to be 
clearly specified.  Maybe this is still OK with the right hints (somewhere?) 
about the filenames that are now not possible to read or create.


Any thoughts on if
jcmd PID command -help
or 
jcmd PID command --help

..should work, or could work?  Additionally, or instead?  Maybe they are 
better, as users relying on files called "-help" would be really uncommon (and 
would be inviting problems!).


Good to ignore the trailing whitespace.
Not obvious whether trailing extra text should be ignored or cause an error.
e.g.
jcmd 7537 VM.stringtable help nonsense
7537:
java.lang.IllegalArgumentException: Unknown argument 'help' in diagnostic 
command.

..where I guess it's not the "help" which is unknown.  Is the user better 
served by just giving the help and ignoring the additional "nonsense"?

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

PR Review: https://git.openjdk.org/jdk/pull/19776#pullrequestreview-2139311805

Reply via email to