Jeff King <peff@xxxxxxxx> wrote: > AFAICT this is only here to avoid having to s/buf/line->buf/ in the rest > of the function. But I think we should just make that change (you > already did in some of the spots). And IMHO we should do the same for > line->len. When there are two names for the same value, it increases the > chances of a bug where the two end up diverging. My motivation was rather to keep patch(es) as small as possible because every line using buf will be replaced in a later patch in series. But it will make commit better (it will stand on its own), so why not to do it? :) > (…) I think short-circuiting like: > > if (!line->len || line->buf[0] == '#') > > is better (I also think "!" instead of "== 0" is our usual style, but > that's much less important). Ah, I only replaced comparison to NULL terminator with length check because I thought it better shows intention of the code and I didn't notice, that reversing order will result in better code overall. I will include both changes in v4. -- | ← Ceci n'est pas une pipe Patryk Obara
