On 12/01/2015 09:44 AM, Lukas Slebodnik wrote:
> On (30/11/15 14:55), Petr Cech wrote:
>> Hi,
>>
>> there is little patch which adds clang-format support. More info is
>> in header
>> of patch.
>>
>> My previous patch set [PATCH SET] TEST_TOOLS_COLONDB: Add tests for
>> sss_colondb_* public API is formatted by clang-format.
>>
>> Regards
>>
>> Petr
>
>> From abfaecb52a65276591e438953f1a3886e166e285 Mon Sep 17 00:00:00 2001
>> From: Petr Cech <pcech(a)redhat.com>
>> Date: Mon, 30 Nov 2015 08:46:35 -0500
>> Subject: [PATCH] CONTRIB: Add clang-format support
>>
>> This patch add support for clang-format.
>>
>> Sometimes we write new source file to codebase. It could be helpfull to
>> use clang-format for autoformating. Or you could use -lines and say
>> which lines should be affected. For more information see
>> clang-format -h
>>
>> Note: If you would like to use clang-format, you need copy file
>> contrib/clang-format/.clang-format to root directory of sssd.
>> ---
>> contrib/clang-format/.clang-format | 57
>> ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 57 insertions(+)
>> create mode 100644 contrib/clang-format/.clang-format
>>
>> diff --git a/contrib/clang-format/.clang-format
>> b/contrib/clang-format/.clang-format
>> new file mode 100644
>> index
>>
0000000000000000000000000000000000000000..f9204273a4321badeb229b280aef88041e738165
>>
>> --- /dev/null
>> +++ b/contrib/clang-format/.clang-format
>> @@ -0,0 +1,57 @@
>> +---
>> +Language: Cpp
> We do not use C++ but ANSI C :-)
Yes, I know that we use ANSI C, C99 respectively. And how I found in
documentation [1], the right option for C is:
# LK_Cpp (in configuration: Cpp) Should be used for C, C++, ObjectiveC,
ObjectiveC++.
[1]
http://llvm.org/releases/3.5.0/tools/clang/docs/ClangFormatStyleOptions.html
>
>> +# BasedOnStyle: LLVM
>> +AccessModifierOffset: -2
>> +ConstructorInitializerIndentWidth: 4
>> +AlignEscapedNewlinesLeft: false
>> +AlignTrailingComments: true
>> +AllowAllParametersOfDeclarationOnNextLine: true
>> +AllowShortBlocksOnASingleLine: false
>> +AllowShortIfStatementsOnASingleLine: false
>> +AllowShortLoopsOnASingleLine: false
>> +AllowShortFunctionsOnASingleLine: All
>> +AlwaysBreakTemplateDeclarations: false
>> +AlwaysBreakBeforeMultilineStrings: false
>> +BreakBeforeBinaryOperators: false
>> +BreakBeforeTernaryOperators: true
>> +BreakConstructorInitializersBeforeComma: false
>> +BinPackParameters: true
>> +ColumnLimit: 80
>> +ConstructorInitializerAllOnOneLineOrOnePerLine: false
>> +DerivePointerAlignment: false
>> +ExperimentalAutoDetectBinPacking: false
>> +IndentCaseLabels: false
>> +IndentWrappedFunctionNames: false
>> +IndentFunctionDeclarationAfterType: false
>> +MaxEmptyLinesToKeep: 1
>> +KeepEmptyLinesAtTheStartOfBlocks: true
>> +NamespaceIndentation: None
>> +ObjCSpaceAfterProperty: false
>> +ObjCSpaceBeforeProtocolList: true
>> +PenaltyBreakBeforeFirstCallParameter: 19
>> +PenaltyBreakComment: 300
>> +PenaltyBreakString: 1000
>> +PenaltyBreakFirstLessLess: 120
>> +PenaltyExcessCharacter: 1000000
>> +PenaltyReturnTypeOnItsOwnLine: 60
>> +PointerAlignment: Right
>> +SpacesBeforeTrailingComments: 1
>> +Cpp11BracedListStyle: true
>> +Standard: Auto
>> +IndentWidth: 4
>> +TabWidth: 4
>> +UseTab: Never
>> +BreakBeforeBraces: Linux
>> +SpacesInParentheses: false
>> +SpacesInAngles: false
>> +SpaceInEmptyParentheses: false
>> +SpacesInCStyleCastParentheses: false
>> +SpacesInContainerLiterals: true
>> +SpaceBeforeAssignmentOperators: true
>> +ContinuationIndentWidth: 4
>> +CommentPragmas: '^ IWYU pragma:'
>> +ForEachMacros: [ foreach, Q_FOREACH, BOOST_FOREACH ]
>> +SpaceBeforeParens: ControlStatements
>> +DisableFormat: false
>> +...
>> +
>
> The fine need's nmore tuning.
>
> It breaks wrapping of DEBUG macros with long debug message.
> - DEBUG(SSSDBG_TRACE_LIBS,
> - "Child request returned [%d]: %s\n", ret, strerror(ret));
> + DEBUG(SSSDBG_TRACE_LIBS, "Child request returned [%d]: %s\n", ret,
> + strerror(ret));
There are two points:
1) I thought that we can wrap long lines by ourselves. My fault.
2) Clang-format has some penalties which weigh the situation. I am
afraid there could be deviations.
>
> There are missing spaces after "{" and before "}"
> - { "dyndns_update", "true" },
> - { "dyndns_refresh_interval", "2" },
> - { NULL, NULL }, /* Sentinel */
> + {"dyndns_update", "true"},
> + {"dyndns_refresh_interval", "2"},
> + {NULL, NULL}, /* Sentinel */
>
> It breaks indentation of "struct poptOption"
> - POPT_AUTOHELP
> - SSSD_DEBUG_OPTS
> - {"no-cleanup", 'n', POPT_ARG_NONE, &no_cleanup, 0,
> - _("Do not delete the test database after a test run"), NULL },
> - POPT_TABLEEND
> - };
> + POPT_AUTOHELP SSSD_DEBUG_OPTS{
> + "no-cleanup", 'n', POPT_ARG_NONE, &no_cleanup, 0,
> + _("Do not delete the test database after a test run"),
> NULL},
> + POPT_TABLEEND};
>
> We allow to have returned type of function on separate line
> -static errno_t
> -sdap_get_members_with_primary_gid(TALLOC_CTX *mem_ctx,
> - struct sss_domain_info *domain,
> - gid_t gid, char ***_localdn, size_t
> *_ndn)
> +static errno_t sdap_get_members_with_primary_gid(TALLOC_CTX *mem_ctx,
> + struct
> sss_domain_info *domain,
> + gid_t gid, char
> ***_localdn,
> + size_t *_ndn)
>
>
> Following change is against our coding guidelines.
> - if (!gid) return EOK;
> + if (!gid)
> + return EOK;
> # single line version is allowed, but multi line version should be part
> # of block {}
>
>
> And they might be more. I tested only two files.
>
> We might fix some issues and ignore others. So it can be used
> for simplification of checking coding style.
>
> I'm not sure we will be able to create the style which
> will 100% comply with our needs.
>
> LS
Thank you for trying, Lukas.
I know that there is another software indent [2] but I have no
experience with it.
I looked at SSSD C coding style [3]. I am afraid there are many rules we
use, but their are not covered in SSSD C coding style.
In my opinion, it could be nice to have tool which simple say your new
code is right by appropriate coding style.
Regards
Petr
PS: I could try improve configuration for clang-format.
[2]
https://www.gnu.org/software/indent/
[3]
http://www.freeipa.org/page/Coding_Style
This is a newer version of the coding style if you want
to take a look (WIP, feel free to modify, needs proof reading)