git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Ramkumar Ramachandra <artagnon@gmail.com>
To: Jeff King <peff@peff.net>
Cc: "Git List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>,
	"René Scharfe" <l.s.r@web.de>
Subject: Re: [PATCH v2] .clang-format: introduce the use of clang-format
Date: Wed, 21 Jan 2015 16:28:00 -0500	[thread overview]
Message-ID: <CALkWK0knxJ5VTJoKhR_t4GS7pfg6PPYox9Srf3bvaX=m+sjqVw@mail.gmail.com> (raw)
In-Reply-To: <20150121204502.GA3287@peff.net>

Jeff King wrote:
> On Wed, Jan 21, 2015 at 12:01:27PM -0500, Ramkumar Ramachandra wrote:
>> +BreakBeforeBraces: Linux
>> [...]
>> +BreakBeforeBraces: Stroustrup

Oh, oops.

>  - It really wants to break function declarations that go over the
>    column limit, even though we often do not do so. I think we're pretty
>    inconsistent here, and I'd be fine going either way with it.
>
>  - It really wanted to left-align some of my asterisks, like:
>
>      struct foo_list {
>        ...
>      } * foo, **foo_tail;
>
>    The odd thing is that it gets the second one right, but not the first
>    one (which should be "*foo" with no space). Setting:
>
>      DerivePointerAlignment: false
>      PointerAlignment: Right
>
>    cleared it up, but I'm curious why the auto-deriver didn't work.

Sounds like a bug.

>  - It really doesn't like list-alignment, like:
>
>       #define FOO    1
>       #define LONGER 2
>
>    and would prefer only a single space between "FOO" and "1". I think
>    I'm OK with that, but we have a lot of aligned bits in the existing
>    code.
>
>  - It really wants to put function __attribute__ macros on the same line
>    as the function. We often have it on a line above (especially it can
>    be so long). I couldn't find a way to specify this.

You have to compromise a bit if you want to use an auto-formatting
tool, without losing your head patching every little detail :)

>  - I had a long ternary operator broken across three lines, like:
>
>      foo = bar ?
>            some_long_thing(...) :
>            some_other_long_thing(...);
>
>    It put it all on one long line, which was much less readable. I set
>    BreakBeforeTernaryOperators to "true", but it did nothing. I set it
>    to "false", and then it broke. Which seems like a bug. It also
>    insisted on indenting it like:
>
>      foo = bar ?
>                    some_long_thing(...) :
>                    some_other_long_thing(...);
>
>     which I found less readable.

To be honest, the LLVM community doesn't fix bugs just because they
can be fixed: it's quite heavily driven by commercial interest. And I
really don't find long ternary operators in a modern C++ codebase.

> I'm slightly dubious that
> any automated formatter can ever be _perfect_ (sometimes
> human-subjective readability trumps a hard-and-fast rule), but this
> seems like it might have some promise.

It works almost perfectly for the LLVM umbrella of projects. When they
want to change a coding convention (like leading Uppercase for
variable names), they write a clang-tidy thing to do it automatically.

> So overall I think it has some promise, but I do not think it is quite
> flexible enough yet for us to use day-to-day.

The big negative is that it will probably never be. I'll try to look
at the larger issues later this week, if you can compromise on the
fine details that are probably too hard to fix.

>   2. It can operate on patches (and generates patches for you to apply!
>      You could add a git-add--interactive mode to selectively take its
>      suggestions).

There's a git-clang-format in the $CLANG_ROOT/tools/clang-format/. I do:

   $ g cf @~

... with the appropriate aliases.

  reply	other threads:[~2015-01-21 21:28 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-17 21:30 [PATCH] .clang-format: introduce the use of clang-format Ramkumar Ramachandra
2015-01-18 11:31 ` René Scharfe
2015-01-21 17:06   ` Ramkumar Ramachandra
2015-01-21 17:01 ` [PATCH v2] " Ramkumar Ramachandra
2015-01-21 20:45   ` Jeff King
2015-01-21 21:28     ` Ramkumar Ramachandra [this message]
2015-01-21 22:09       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALkWK0knxJ5VTJoKhR_t4GS7pfg6PPYox9Srf3bvaX=m+sjqVw@mail.gmail.com' \
    --to=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).