On 7/6/19 8:55 PM, Yasumasa Suenaga wrote:
Hi Chris,
Thank you for your advise.
I uploaded new webrev. How about this?
http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.04/
I replaced "server" to "host" as you said.
Other comments from you also have been fixed in it.
For example, `jhsdb jstack --help` shows as below:
```
$ jhsdb jstack --help
--locks To print java.util.concurrent locks.
--mixed To print both Java and native frames
(mixed mode).
--pid <pid> To attach to and operate on the given live
process.
--core <corefile> To operate on the given core file.
--exe <executable for corefile>
--connect <[id@]host> To connect to a remote debug server
(sadebugd).
The --core and --exe options must be set together to give the core
file, and associated executable, to operate on. They can use
absolute or relative paths.
The --pid option can be set to operate on a live process.
The --connect option can be set to connect to a debug server
(sadebugd).
<--core --exe> and <--pid> and <--connect> are exclusive.
Perhaps:
s/are exclusive./are mutually exclusive./
would be a little more clear.
Dan
Examples: jhsdb jstack --pid 1234
or jhsdb jstack --core ./core.1234 --exe ./myexe
or jhsdb jstack --connect debugserver
or jhsdb jstack --connect id@debugserver
```
If you are ok, I will reopen CSR (JDK-8224979).
Yasumasa
On 2019/07/06 6:26, Chris Plummer wrote:
On 7/4/19 3:53 PM, Yasumasa Suenaga wrote:
Hi Chris,
On 2019/07/05 4:24, Chris Plummer wrote:
Hi Yasumasa,
On 7/4/19 5:30 AM, Yasumasa Suenaga wrote:
Hi Chris,
Thank you for your review.
On 2019/07/04 8:07, Chris Plummer wrote:
Hi Yasumasa,
Overall these changes look good, but I think there is a bit of
cleanup still needed.
You've changed the indentation of the help to always have a few
spaces before the /t. If you are going to do this, then perhaps
you don't need the /t anymore.
Ok, I will replace '\t' to whitespace.
There are 3 checks for "remote != null". Shouldn't they be
"remote != NO_REMOTE"?
I will fix them.
The old jstack help output is a bit more informative than your
output:
jstack [-m] [-l] [server_id@]<remote server IP or hostname>
(to connect to a remote debug server)
vs
--connect <[id@]server> To operate on the remote debug server.
More specifically, you replaced <remote server IP or hostname>
with "server". I think the former is a bit more self explanatory.
Exactly, but it is too long to show on the console.
So I used short word: server.
If we can ignore long line, or can add newline (\n) on help message,
I can change it. What do you think?
I don't see why not. Isn't there already help output that relies on
newlines?
As you can see on the CSR (JDK-8224979), format of jhsdb help
messages is:
<option name> <blank (includes tab)> <description>
Thus I think long option / description is not comfortable.
If we need to describe more, I think we should write it as below:
--connect <[server-id@]remote server IP or hostname>
To connect to a remote debug server.
Hi Yasumasa,
We already have multiline examples like:
|--pid <pid> To attach to and operate on the given live process.
Is your concerned that the long option name will extend into the
columns where the description normally is? If so, you can start the
description on a new line as you suggest, or maybe come up with a
different name for the option that isn't quite as long. My main
objection to just saying "server" is it in no way conveys we are
talking about an IP host. Even just saying <host> would be better. It
is also more correct since the host isn't necessarily what someone
would consider to be a server (it could just be someone's desktop).
<server-id> on the other hand is the correct term because we are
talking about the id of the debug server running on a particular host.
Chris
|
