On Thu, 8 Aug 2024 18:45:00 GMT, Kevin Walls <kev...@openjdk.org> wrote:

>> src/jdk.jcmd/share/man/jcmd.1 line 258:
>> 
>>> 256: \f[I]filename\f[R]: (Optional) The name of the map file.
>>> 257: If %p is specified in the filename, it is expanded to the JVM\[aq]s 
>>> PID.
>>> 258: (FILE, \[dq]/tmp/perf-%p.map\[dq])
>> 
>> I'm not sure about this change to FILE from STRING. Are there previous uses 
>> of FILE? We still seem to use STRING for JFR.dump, JFR.start, and JFR.stop.
>
> OK yes - my thinking is:
> 
> They are strings of course, but they are also names of an output FILE, and 
> those are distinct because they do %p substitution.
> 
> They need to have a "type" of FILE internally in the tool, so they get %p 
> substitution.
> 
> The FILE instead of STRING doesn't need to be in the man page, but it would 
> make sense that the man page is the same as the live help output as much as 
> possible.
> 
> After the intro, the body of the man page mostly duplicates the online help 
> for the commands.
> So much so that you'd think we can automate that, which would make a great 
> future step.
> Then we would get FILE for the type of these arguments, not STRING.  So I 
> used FILE here as it avoids a future issue.
> 
> I think we should move other output file names e.g. in the JFR commands to 
> FILE over time, after verifying they do the %p substitution.  There could be 
> %t timestamp as well, I think an enhancement request for that exists.
> 
> Then the docs could summarise everything about the FILE parameters and not 
> have to duplicate it every time.

Ok. I see now that [JDK-8334492](https://bugs.openjdk.org/browse/JDK-8334492) 
made the STRING -> FILE change, so yes the docs should be consistent, but I 
think we need a new CR filed to cleanup JFR dcmds, which continue to use 
STRING. This probably should have been done with 
[JDK-8334492](https://bugs.openjdk.org/browse/JDK-8334492).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20401#discussion_r1711887119

Reply via email to