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

[ng0]

address@hidden transcribed 10K bytes:
> 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).

Though it is very likely an issue with current clang-format,
as it operates on tokens if I remember correctly. It is
reasonable to assume that what I did there is bad or
uncommon style and clang-format considers code between
directives.
  
> > 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
> > > ...
> > 
> > 
> > 
> > 
> > 
>