[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8254861: reformat code in vmTestbase/nsk/jdb [v2]
From:       Chris Plummer <cjplummer () openjdk ! java ! net>
Date:       2020-10-22 15:43:12
Message-ID: KW6sABNHmJZarW4Z7c8pr17GwphLkxhHido-gdxO6uU=.8991932d-4a11-47e9-8530-c7cf30163add () github ! com
[Download RAW message or body]

On Thu, 22 Oct 2020 08:48:20 GMT, Serguei Spitsyn <sspitsyn@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


[prev in list] [next in list] [prev in thread] [next in thread]