Github user alanconway commented on the issue:
https://github.com/apache/qpid-proton/pull/169
On Wed, Nov 28, 2018 at 12:29 PM Andrew Stitcher <[email protected]>
wrote:
> *I 100% support having a consistent automated format for the proton
> codebase(s)*
>
> However , I don't think this is quite it (IMO):
>
Have a bash: `clang-format -style=X -dump-style` will give you all the
gory details, also check the clang site for the list of pre-defined styles
(Mozilla and others), you may find one more to your liking.
>
> - I think we should just leave the proton-c codebase with an indent of
> 2 - A lot of changes for marginal gain there.
> - We can have a 4 space indent for the C++ code, I don't think
> having slightly different settings is a problem if the format is
> automatically applied.
>
> Easy to do, just put separate .clang-format files in /c and /cpp . I'd
prefer a common style for both however. The first big reformat will be a
pain (mitigated by automated reformatting for outstanding commits) but the
pain will pass.
If we use different styles for C and C++, should C++ files in the C tests
tree be formated like C or like the C++ binding? The latter will make
automated reformatting more complicated.
>
> - I think we can allow wider lines - it looks like it is wrapping at
> 80 chars, I think 100 would be fine (conservative even) for modern
screens.
>
> +1, I went with minimal changes from LLVM but 100 is fine, I find 80 a
bit tight.
>
> - I'm not happy with the reformatting of the header files:
> - Especially reformatting the 2nd and subsequent parameter at a
> variable indent - not cool for variable width fonts!.
>
> No space-based alignment method works for variable width fonts. Are you
proposing we move to tab-based indentation, or to a style that never aligns
anything with the previous line? I'm pretty sure clang-format can do that,
just not sure if that's what you mean.
>
> -
> - Seems to have reformatted blah* x(); to blah *x(); which is fine
> for C code, but not for C++ code. Another area where the setting
should
> probably be different.
>
> Depends who you ask, lots of C++ style guides use blah *x; as it more
accurately reflects the precendence of *.
For example `blah *x, *y` means what it looks like; `blah* x, y` does not
(y is a blah, not a blah*)
If you want this I believe the clang-format variable is `PointerAlignment:
Left`. Again I'd prefer we be consistent in C and C++.
> I think the mess of the header files is actually the most important issue
> given that this is user visible documentation. It should look as nice as
we
> can make it. Consistency is important, but it shouldn't be ugly to view -
I
> currently opine that the reformat has made the header files ugly.
>
I went with adopting an existing style guide with minimal changes, but no
objection if you can find or craft a more beautiful one.
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/qpid-proton/pull/169#issuecomment-442533870>,
> or mute the thread
>
<https://github.com/notifications/unsubscribe-auth/AHa6XvLNnwh7UBpAiU29hDQB71oGvpGKks5uzsgGgaJpZM4Y2Wqk>
> .
>
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
