On Mon, Aug 14, 2017 at 04:47:45PM -0700, Junio C Hamano wrote:
> > 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 ;-)
Yep. Our solutions were at opposite ends of the spectrum, though. :)
> > 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.
Yes, agreed. My personal rule with a list like this is often "once you
have to start breaking across multiple lines, you should put one per
line". I don't know if there's a way to codify that in clang-format,
though.
The case I fed it (which is just nonsense I made up that does not fit
our style) also left me a bit confused at first, but I think it was
because the .clang-format parser was bailing as soon as it found an
unrecognized entry, but then formatting according to bogus rules. With
the original file from Brandon I got:
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 7e8371670b..8994450e0c 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -21,10 +21,7 @@ static int label_cb(const struct option *opt, const char *arg, int unset)
return 0;
}
-static
-int foo(void* bar,int baz) {
- /* nothing */
-}
+static int foo(void *bar, int baz) { /* nothing */ }
int cmd_merge_file(int argc, const char **argv, const char *prefix)
{
which is clearly not our style. And then after removing the entries I
mentioned elsewhere, I get:
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 7e8371670b..574ba6d86f 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -21,8 +21,8 @@ static int label_cb(const struct option *opt, const char *arg, int unset)
return 0;
}
-static
-int foo(void* bar,int baz) {
+static int foo(void *bar, int baz)
+{
/* nothing */
}
which looks right. So you might want to double-check that it was
respecting our settings, and there were no warnings to stderr.
-Peff
