On Tue, Jun 21, 2016 at 2:31 PM James Peach <jpe...@apache.org> wrote:
>
> > On Jun 21, 2016, at 12:19 PM, Phil Sorber <sor...@apache.org> wrote:
> >
> > So I was +1 on this, but now looking at the full diff I am -0. I wanted
> to
> > get others take on this. It aligns *all* assignments not just declaration
> > ones.
> >
> > For example:
> > - c->vio.op = VIO::READ;
> > + c->vio.op = VIO::READ;
> > c->base_stat = cache_lookup_active_stat;
> > CACHE_INCREMENT_DYN_STAT(c->base_stat + CACHE_STAT_ACTIVE);
> > c->first_key = c->key = *key;
> > - c->frag_type = type;
> > - c->f.lookup = 1;
> > - c->vol = vol;
> > - c->last_collision = NULL;
> > + c->frag_type = type;
> > + c->f.lookup = 1;
> > + c->vol = vol;
> > + c->last_collision = NULL;
> >
> > This seems weird and distracting to me. It groups ones on consecutive
> lines
> > too. So you might have different spacing within one body of code because
> > there are other non-assignment lines in between. It also doesn't appear
> to
> > align things like +=.
> >
> > - cstate->cache_vc = (TSVConn)edata;
> > - cstate->write_vio = TSVConnWrite(cstate->net_vc, contp,
> > cstate->resp_reader, INT64_MAX);
> > + cstate->cache_vc = (TSVConn)edata;
> > + cstate->write_vio = TSVConnWrite(cstate->net_vc, contp,
> > cstate->resp_reader, INT64_MAX);
> > cstate->total_bytes += TSIOBufferWrite(cstate->resp_buffer, error,
> > sizeof(error) - 1);
> >
> > If everyone is still onboard then lets stick with it, but just wanted to
> > talk this out.
>
> Yeh I agree that there are ugly cases, but on balance I think this made
> the formatting cleaner. With some judicious blank lines to reset the
> indentation and application of clang-format-off, can we improve the
> distracting cases you found?
>
I think the distracting cases I found were due to reset indentation.
Ignoring the augmented assignment won't be fixed either.
I think if everyone is on board, I can make do. Just wanted to have this
conversation.
Thanks.
>
> >
> > Thanks.
> >
> > On Sun, Jun 19, 2016 at 9:17 PM Leif Hedstrom <zw...@apache.org> wrote:
> >
> >>
> >>> On Jun 17, 2016, at 9:46 AM, James Peach <jpe...@apache.org> wrote:
> >>>
> >>>>
> >>>> On Jun 16, 2016, at 2:04 PM, Leif Hedstrom <zw...@apache.org> wrote:
> >>>>
> >>>> Hi all,
> >>>>
> >>>> James and I would like to make this small, but powerful, change to
> >> clang-format:
> >>>>
> >>>> -AlignConsecutiveAssignments: false
> >>>> +AlignConsecutiveAssignments: true
> >>>>
> >>>>
> >>>> This will try to align assignments such that the = signs line up. E.g.
> >>>>
> >>>> enum {
> >>>> XHEADER_X_CACHE_KEY = 0x0004u,
> >>>> XHEADER_X_MILESTONES = 0x0008u,
> >>>> XHEADER_X_CACHE = 0x0010u,
> >>>> XHEADER_X_GENERATION = 0x0020u,
> >>>> XHEADER_X_TXN_UUID = 0x0040u,
> >>>> };
> >>>
> >>> +1 from me.
> >>
> >>
> >>
> >> I have committed this. One side effect of this is that all PR’s likely
> >> will have to be updated with a new “make clang-format”.
> >>
> >> Cheers,
> >>
> >> — leif
> >>
> >> P.s
> >> One thing to be aware of, the alignment on the ‘=‘ resets if you have an
> >> empty line or a comment in the middle of e.g. an enum, or list of
> >> assignments in general.
>
>
