Re: [libmicrohttpd] clang formatting discussion via [gnunet-developers]

[ng0]

address@hidden transcribed 9.3K bytes:
> Christian Grothoff transcribed 11K bytes:
> > Dear MHD hackers,
> > 
> > There has been a (long-ish) discussion about using clang-format for
> > source formatting among the GNUnet developers.  WDYT about applying the
> > same/similar rules to GNU libmicrohttpd and also enforcing them via Git
> > hooks on commit? I've attached the .clang-format file that resulted from
> > the GNUnet discussion here.
> 
> Did you reach any consensus on this discussion here?
> I'm already clang-formatting my code for lmhd but no
> .clang-format exists in the repository.

I've noticed another issue in clang-format from llvm-project.git
54763e44532feb72bf773260b377f2a0bacf0e0c and base llvm+clang+... version 7.0:

If you have a couple of preprocessor #if directives, and you conditionally
close if conditions not in the same #if block, clang-format assumes the
if conditions are never closed.

example:

#if HAVE_FOO
if (something) {
        foo;
        if (otherthing) {
                foo;
#endif
#if HAVE_BAR
if (newthing) {
        bar;
        if (newthing) {
                bar;
#endif
} else {
        errx(1, "Error: %s", bla);
}
}


This could be an error with clang-format, or my mixture
of versions (which otherwise works alright).
 
> If you require an entirely different style,
> contributing to LibFormat and clang-format is an option.
> I plant to do this with the outstanding KNF for NetBSD.
> 
> As far as I know we weren't 100% okay with the results
> for gnunet but already use it.
>  
> > See in particular the following e-mails from the GNUnet discussion:
> > 
> > 
> > On 4/15/19 10:02 AM, Schanzenbach, Martin wrote on [GNUnet-developers]:> Hi,
> > >
> > > FYI I added a clang-format at "contrib/conf/editors/clang-format".
> > > Clang-format is usable with most editors (vim, emacs, vscode) with
> > respective plugins.
> > > This way, we can have a unified coding style.
> > > Clang-format has a nice documentation and allows fine-grained setting
> > which are also human readable:
> > https://clang.llvm.org/docs/ClangFormatStyleOptions.html
> > >
> > > Let's use it and put it as a note into our coding guidelines?>
> > 
> > Martin also wrote:
> > 
> > > Most editors have plugins:
> > >
> > > emacs:
> > https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/clang-format.el
> > > (n)vim: https://github.com/rhysd/vim-clang-format
> > > vscode: native
> > > sublime: https://packagecontrol.io/packages/Clang%20Format
> > 
> > And most recently I wrote (latest point of the discussion):
> > 
> > >
> > > Yes, the question is this: are there any developers here where their
> > > editor cannot be reasonably integrated with clang-format? If so, please
> > > do shout out now!
> > >
> > > I've so far heard only positive feedback for Martin's overall initiative
> > > to get indentation under control, so for now I'll proceed with the
> > > assumption that there are no blockers. But if you do have an issue with
> > > this, please do let me know (on or off list) as soon as possible. If
> > > there are no (sound) objections soon, I will give ng0/schanzen the
> > > go-ahead to enable enforcement of clang formatting for all Git commits
> > > in the future!
> > >
> > > Anyway, aside from the general policy debate, please read on for more
> > > specific related issues.
> > >
> > > //// Deployment experience
> > >
> > > I had to install clang-format-9 from Debian experimental, as our
> > > configuration includes options that are only in v9.
> > >
> > > For Emacs user's reference: Without v9, the Emacs integration silently
> > > failed. I also had to create a symlink from clang-format to
> > > clang-format-9, as the Emacs integration only looks for the clang-format
> > > binary (and otherwise also fails silently).
> > >
> > >
> > > Other than that, Emacs integration was straightforward:
> > >
> > > I added to gnunet/.dir-locals.el a hook on save (this may end up in Git
> > > "soon")
> > >
> > >>>>
> > > ((c-mode
> > >   (eval add-hook 'before-save-hook #'clang-format-buffer nil t)))
> > > <<<
> > >
> > >
> > > and to .emacs the logic to find the clang-format package
> > >
> > >>>>
> > > (require 'package)
> > > (add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/";))
> > > ;; initialize package system
> > > (package-initialize)
> > > ;; actually load the package
> > > (require 'clang-format)
> > > <<<
> > >
> > > And of course also installed the clang-format package (interactively,
> > > M-x install-package) as per documentation from Martin's first link above.
> > >
> > > Finally, the clang-format style symlink must be set:
> > >
> > > # Install clang format symlink
> > > ln -s contrib/conf/editors/clang-format .clang-format
> > >
> > > (this I will at to the bootstrap script).
> > >
> > >
> > > //// Formatting gripes
> > >
> > > My remaining main issue with
> > > https://clang.llvm.org/docs/ClangFormatStyleOptions.html
> > > is that it cannot currently be forced to put a break after each argument
> > > and each parameter, and it also cannot detect the exception that a
> > > function has key-value pairs and thus should keep the arguments paired.
> > >
> > > However, the latter case is solvable! If we have key-value pairs, we
> > can do:
> > >
> > > /* clang-format off */
> > > fun (generalargs,
> > >      key1, value1,
> > >      key2, value2,
> > >      key3, value3);
> > > /* clang-format on */
> > >
> > > Real-world example from gnunet-gtk/src/namestore/gnunet-namestore-gtk.c:
> > >
> > >  /* clang-format off */
> > >  gtk_tree_model_get (tm, &iter,
> > >                      GNS_TREESTORE_COL_RECORD_TYPE, &n_type,
> > >                      GNS_TREESTORE_COL_IS_PUBLIC, &n_public,
> > >                      GNS_TREESTORE_COL_EXP_TIME, &n_exp_time,
> > >                      GNS_TREESTORE_COL_EXP_TIME_IS_REL, &n_is_relative,
> > >                      GNS_TREESTORE_COL_IS_SHADOW, &n_is_shadow,
> > >                      GNS_TREESTORE_COL_VAL_AS_STR, &n_value,
> > >                      -1);
> > >  /* clang-format on */
> > >
> > > That way, clang-format won't touch the *nice* formatting. So if there is
> > > a very strong reason, I would definitively approve turning off clang for
> > > parts of the source.
> > >
> > > For this reason, please do NOT simply apply clang-format globally to
> > > GNUnet.  Instead, when editing a file, do a quick check to see if
> > > clang-format would destroy something that's just done "right", and *turn
> > > it off* for those regions.  After that, a single commit with just the
> > > clang-forward should be OK.
> > >
> > >
> > > Happy hacking!
> > >
> > > Christian
> > >
> > 
> > Please let me know what you think of this.
> > 
> > Happy hacking!
> > 
> > Christian
> 
> > # This file is in the public domain.
> > ---
> > Language:        Cpp
> > # BasedOnStyle:  LLVM
> > AccessModifierOffset: -2
> > AlignAfterOpenBracket: Align
> > AlignConsecutiveAssignments: false
> > AlignConsecutiveDeclarations: false
> > AlignEscapedNewlines: Left
> > AlignOperands:   true
> > AlignTrailingComments: false
> > 
> > AllowAllArgumentsOnNextLine: false
> > 
> > AllowAllParametersOfDeclarationOnNextLine: false
> > AllowShortBlocksOnASingleLine: false
> > AllowShortCaseLabelsOnASingleLine: false
> > AllowShortFunctionsOnASingleLine: All
> > AllowShortIfStatementsOnASingleLine: false
> > AllowShortLoopsOnASingleLine: false
> > AlwaysBreakAfterDefinitionReturnType: None
> > AlwaysBreakAfterReturnType: TopLevel
> > AlwaysBreakBeforeMultilineStrings: false
> > AlwaysBreakTemplateDeclarations: MultiLine
> > BinPackArguments: false
> > BinPackParameters: false
> > BraceWrapping:
> >   AfterClass:      false
> >   AfterControlStatement: true
> >   AfterEnum:       true
> >   AfterFunction:   true
> >   AfterNamespace:  false
> >   AfterObjCDeclaration: false
> >   AfterStruct:     true
> >   AfterUnion:      true
> >   AfterExternBlock: false
> >   BeforeCatch:     false
> >   BeforeElse:      true
> >   IndentBraces:    false
> >   SplitEmptyFunction: true
> >   SplitEmptyRecord: true
> >   SplitEmptyNamespace: true
> > BreakBeforeBinaryOperators: None
> > BreakBeforeBraces: Custom
> > BreakBeforeInheritanceComma: false
> > BreakInheritanceList: BeforeColon
> > BreakBeforeTernaryOperators: true
> > BreakConstructorInitializersBeforeComma: false
> > BreakConstructorInitializers: BeforeColon
> > BreakAfterJavaFieldAnnotations: false
> > BreakStringLiterals: false
> > ColumnLimit:     80
> > CommentPragmas:  '^ IWYU pragma:'
> > CompactNamespaces: false
> > ConstructorInitializerAllOnOneLineOrOnePerLine: false
> > ConstructorInitializerIndentWidth: 2
> > ContinuationIndentWidth: 2
> > Cpp11BracedListStyle: true
> > DerivePointerAlignment: false
> > DisableFormat:   false
> > ExperimentalAutoDetectBinPacking: true
> > FixNamespaceComments: true
> > ForEachMacros:
> >   - foreach
> >   - Q_FOREACH
> >   - BOOST_FOREACH
> > IncludeBlocks:   Preserve
> > IncludeCategories:
> >   - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
> >     Priority:        2
> >   - Regex:           '^(<|"(gtest|gmock|isl|json)/)'
> >     Priority:        3
> >   - Regex:           '.*'
> >     Priority:        1
> > IncludeIsMainRegex: '(Test)?$'
> > IndentCaseLabels: false
> > IndentPPDirectives: None
> > IndentWidth:     2
> > IndentWrappedFunctionNames: false
> > JavaScriptQuotes: Leave
> > JavaScriptWrapImports: true
> > KeepEmptyLinesAtTheStartOfBlocks: true
> > MacroBlockBegin: ''
> > MacroBlockEnd:   ''
> > MaxEmptyLinesToKeep: 2
> > NamespaceIndentation: None
> > ObjCBinPackProtocolList: Auto
> > ObjCBlockIndentWidth: 2
> > ObjCSpaceAfterProperty: false
> > ObjCSpaceBeforeProtocolList: true
> > PenaltyBreakAssignment: 2
> > PenaltyBreakBeforeFirstCallParameter: 9999999
> > PenaltyBreakComment: 300
> > PenaltyBreakFirstLessLess: 120
> > PenaltyBreakString: 1000
> > PenaltyBreakTemplateDeclaration: 10
> > PenaltyExcessCharacter: 1000000
> > PenaltyReturnTypeOnItsOwnLine: 60
> > PointerAlignment: Right
> > ReflowComments:  true
> > SortIncludes:    false
> > SortUsingDeclarations: true
> > SpaceAfterCStyleCast: true
> > 
> > SpaceAfterLogicalNot: true
> > 
> > SpaceAfterTemplateKeyword: true
> > SpaceBeforeAssignmentOperators: true
> > SpaceBeforeCpp11BracedList: false
> > SpaceBeforeCtorInitializerColon: true
> > SpaceBeforeInheritanceColon: true
> > SpaceBeforeParens: Always
> > SpaceBeforeRangeBasedForLoopColon: true
> > SpaceInEmptyParentheses: false
> > SpacesBeforeTrailingComments: 1
> > SpacesInAngles:  false
> > SpacesInContainerLiterals: true
> > SpacesInCStyleCastParentheses: false
> > SpacesInParentheses: false
> > SpacesInSquareBrackets: false
> > Standard:        Cpp11
> > StatementMacros:
> >   - Q_UNUSED
> >   - QT_REQUIRE_VERSION
> > TabWidth:        2
> > UseTab:          Never
> > ...
> 
> 
> 
> 
>