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 Hi all, Thanks for the comments. Some thoughts: > what about '*jcmd <pid> help <cmd>*' > not a great deal of additional code for the 'help' jcmd to delegate to > the target jcmd? ```jcmd <pid> help <cmd>``` is already supported. This patch aims to rearrange the arguments provided to leverage what you suggested (i.e. ```jcmd <pid> <cmd> help``` -> ```jcmd <pid> help <cmd>```). Not sure if I misunderstood your message though :-) Regarding the current state of the patch, > 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. This is a really good point and in my opinion the strongest argument to use `--help` or `-help` instead. My only pushback would be whether it would be a bit inconsistent to make it so that the dashes need to be specified for help in this case ```jcmd <pid> <cmd> -help```, but not in this one ```jcmd <pid> help <cmd>```. Unless we plan to rework “help” in general and make it so that you always need the dashes regardless of how it gets invoked. > > 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"? Sure, that makes sense to me. I can make an update to ignore additional nonsense once we agree on the other points. As a final question, if we make it so we need the dashes to enable help, would we need to document this anywhere or do we assume it's self-explanatory as it follows the Posix utility argument syntax that Thomas linked? Thanks for all the feedback! ------------- PR Comment: https://git.openjdk.org/jdk/pull/19776#issuecomment-2192512060
