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