taler
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Taler] Coding style clang-format


From: Florian Dold
Subject: Re: [Taler] Coding style clang-format
Date: Thu, 18 Apr 2019 14:37:50 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1

I think it's a great idea!  Since clang-format can reformat all of
GNUnet in <1s on my laptop, it should be fast enough to run as part of
every commit.

However, there must be an option to enable/disable this on a
per-repository or even per-path basis, since we do not want to re-format
third party code that is checked into our repos.

- Florian

On 4/18/19 2:16 PM, Christian Grothoff wrote:
> Dear Taler hackers,
> 
> There has been a (long-ish) discussion about using clang-format for
> source formatting among the GNUnet developers.  WDYT about applying the
> same rules to GNU Taler and also enforcing them via Git hooks on commit?
> 
> 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
>>
> 

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]