Subject: Re: [PATCH v2 2/2] Makefile: add style build rule

From: Brandon Williams <[email-masked]>
Date: Mon, 14 Aug 2017 17:05:33 -0700

On 08/14, Junio C Hamano wrote:
> Junio C Hamano <[email-masked]> writes:
> 
> > By the way, I do not know which vintage of /usr/bin/git-clang-format
> > I happen to have on my box, but I needed a crude workaround patch
> > (attached at the end) ...
> 
> I guess you hit the same thing while our messages crossing ;-)
> 
> 
> > As to what it does, the first example I tried may not have been a
> > great one.  I got this:
> >
> >         git clang-format --style file --diff --extensions c,h
> >         diff --git a/cache.h b/cache.h
> >         index 73e0085186..6462fe25bc 100644
> >         --- a/cache.h
> >         +++ b/cache.h
> >         @@ -1498,11 +1498,8 @@ struct checkout {
> >                 const char *base_dir;
> >                 int base_dir_len;
> >                 struct delayed_checkout *delayed_checkout;
> >         -	unsigned force:1,
> >         -		 quiet:1,
> >         -		 not_new:1,
> >         -		a_new_field:1,
> >         -		 refresh_cache:1;
> >         +	unsigned force : 1, quiet : 1, not_new : 1, a_new_field : 1,
> >         +		refresh_cache : 1;
> >          };
> >          #define CHECKOUT_INIT { NULL, "" }
> >  
> > which is not wrong per-se, but I have a mixed feelings.  I do not
> > want it to complain if the original tried to fit many items on a
> > single line, but if the original wanted to have one item per line,
> > I'd rather see it kept as-is.
> 
> To clarify, the above is after I added a_new_field that is one-bit
> wide without doing anything else.  I do not mind the checker
> complaining the existing force, quiet, etc. whose widths are all
> spelled without SP around ':', because they appear near-by, as a
> collateral damage.  My only gripe is that the result got squished
> into a single line.

Yeah the result doesn't look too good and I'm not sure which option to
tweak for that.  I'm sure that the problem would fix itself if all the
bit fields where defined on their own lines:

  unsigned force : 1;
  unsigned not_new : 1; 
  ... etc ...

I'm sure there are a bunch of other things that we'd need to tweak
before this is ready to be used by all contributors.  Specifically the
penalties to help determine when to break a line.

> 
> > Anyway, we cannot have perfect checker from the day one, and
> > considering this is an initial attempt, I'd say it is a good start.

-- 
Brandon Williams