← Back to team overview

yade-dev team mailing list archive

Re: clang-format (Was: Re: Removing trailing white space, yay or nay?)

 

Bruno Chareyre said:     (by the date of Mon, 21 Oct 2019 11:57:28 +0200)

> Hi Anton,
> It's not yet all clear to me how it will work.
> What would be the workflow in general for, let's say, a kdevelop/kate user?
> Edit, then "git-clang-format" before commit?

I pushed the script `scripts/clang-formatter.sh` which you can call on
file or a directory and it will do the formatting. The script checks
beforehand if there are any uncommitted changes, so that you do not
mix substantial changes with formatting changes. Mixing them in
single commit might cause problems with reading history, I suppose.

> I'm particularly curious about your statement in the giltab thread [1]:
> "Reformatting everything is not the best idea. It will change everything
> and can hurt the history. I would propose to do it step by step, only by
> changing the files."
> 
> Do you suggest that each time a file will be modified it should be also
> re-formatted, but no systematic reformatting would be done beyond that?
> Isn't it maximizing the risk of mixing real changes with formatting? I
> would think reformatting everything in one single commit would make the
> history much more clear.

Honestly I don't know. I think that at least for start we should just
try to get comfortable with the new system. If we agree that it works for
everyone then maybe we can reformat everything.

clang-format appears to be a quite popular tool. I figured out how to
invoke it from inside vim on the selected text [1]. I am pretty sure you
can do the same from kdevelop/kate.

 
> Last thing I'm thinking about: how will we avoid that different
> authors with (even slightly) different editing tools and
> auto-formating settings end up in committing conflicting
> auto-formats?

The basic assumption is that once we have a .clang-format file, the
script `scripts/clang-formatter.sh` produces the same results for everyone.

So even if you mix the formatting somehow, this script will reset it
to what is in the repository.

However I didn't yet check if the results on ubuntu 16.04 are the
same as on debian bullseye. We better check that before a
full-rollout.

Janek

[1] https://gitlab.com/yade-dev/trunk/merge_requests/298#note_232967339


Follow ups

References