On Thu, 22 Oct 2020 08:48:20 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:

>> File test/hotspot/jtreg/vmTestbase/nsk/jdb/kill/kill001/kill001.java has 
>> this change:
>> 
>>          for (int i = 0; i < threads.length; i++) {
>>              reply = jdb.receiveReplyForWithMessageWait(JdbCommand.kill + 
>> threads[i] + " " +
>> -                                                       DEBUGGEE_EXCEPTIONS 
>> + "[" + i + "]",
>> -                                                       "killed");
>> +                            DEBUGGEE_EXCEPTIONS + "[" + i + "]",
>> +                    "killed");
>>          }
>> I think, the second line "killed");  has to be aligned with the previous one.
>> Also, I feels like this change makes the code to be less readable:
>>          reply = jdb.receiveReplyForWithMessageWait(JdbCommand.eval + 
>> DEBUGGEE_RESULT,
>> -                                                   DEBUGGEE_RESULT + " =");
>> +                DEBUGGEE_RESULT + " =");
>
> Hi Igor,
> 
> Overall, it is great. Your formatting tool seems to be AI. 👍 
> This update fixes a lot of formatting issues.
> I have no more comments so far.

Regarding the "killed" formatting, I think the reason for it is the 8 space 
"line continuation rule". The 2nd line is a actually a line continuation of a 
line continuation, so it gets indented and extra 16 spaces. The 3rd line is 
just a single line continuation, so gets indented just an extra 8. I think what 
would help to clean this up is to move the first argument onto a newline, which 
would just be indented an extra 8. That way it won't be broken up over multiple 
lines, and will also be inline with the "killed" argument.

For the 2nd case above, I also think this is less readable than the original. I 
personally like it if you align all arguments with the first one, even if it 
leads to a non-standard indentation. If you want subsequent arguments to use 
the 8 space line continuation rule, then I suggest whenever arguments are on 
multiple lines that you always place the first argument on a new line so all 
arguments are aligned together.

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

PR: https://git.openjdk.java.net/jdk/pull/689

Reply via email to