PING: RFR: 8209790: SA tools not providing option to connect to debug server

[Chris Plummer]

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
|