auto-indent research result and proposal

Dirk Hohndel dirk at hohndel.org
Sat Jan 18 07:35:42 UTC 2014


On Sat, 2014-01-18 at 11:28 -0200, Danilo Cesar Lemes de Paula wrote:
> So, I've been checking ways to auto indent code before commiting and I
> used subsurface as a guinea pig for my tests with clang-format.
> 
> The approach I took is the same that gstreamer uses, using a git hook to
> check all files that are being commited and run clang-format against them.
> This means that all checked files will have to follow the same rules
> (although we can tweak the hook to add exceptions). So, before a commit,
> your whole file needs to pass the rules, otherwise you'll see a warning
> (and this warning remains until the file is fixed by someone)

You see a warning or the commit isn't run?

> Basically, clang-format needs a .clang-format file in the root folder,
> containing the rules. I started a rules file [1] that I think would make
> sense (following Dirk's comments and a few more patters I saw people using).
> I'm also sending a diff file [2] as a preview of what it can do.
> 
> I enjoyed the final result and I'm probably going to use in other projects.
> But it's not perfect, I have a list of flaws:
> 
> 1 - header files with classes: is common to use public:/private: at the
> begging of the line. clang indents it with a tab.

Yuck - that's really ugly.

> 2 - if(something) {} will be replaced by if (something) {
>     } - it's cool. However, it can't recognize Qt controls statements
> (Q_FOREACH) and assumes it's a function. so it will remove the space and
> put a line break before the brace as in:
>     q_foreach(something, something)
>     {
>       ....
>     }

Not cool

> 3 - it fixes all the arithmetics operators with a space and uses the
> form QString* str instead of QString *str everywhere. However, inside
> the non recognized control structures (q_foreach again) it thinks that
> it's an arithmetic operator and expects the following usage:
>     q_foreach(QString * str, list)... not cool as foreach(QString* str)

I really don't like either.

type *variable;


> 4 - it uses and expect tabstop=8 (kernel's guide expects the same), but
> escaped new lines will look unindented if you use tabstop!=8.

I don't understand that.

> 5 - I couldn't find a way to put space after a cast, so casts will looks
> like:
>     (struct dive*)index.data(); instead of (struct dive*) index.data(),
> which would be easier to see. IHMO.

That one I don't care as much about

> 6 - I never used git-hook before but I don't think it can be included in
> the git repository. What gstreamer does, as example, is to install the
> hook during the ./configure phase. I think we could do the same with qmake.

I haven't looked into this myself, but given that this would only be
needed for developers I'm sure we can figure out how to install
things...

> Even with those issues, I think it's a low price to be paid for
> coding-style consistency in the whole code make the developer's life easier.

Not sure. If every instance of public/private in a .h files creates a
warning and the Qt control structures aren't recognized... that's a non
starter for me.
Can the warnings be post-processed or otherwise filtered.

> Again, it's only a proposal and it's understandably if it doesn't fit
> subsurface's needs.

I think it may be a good start, but as I said, without some ability to
fix a couple of the glaring issues I think it won't solve the issue I'm
trying to solve...

> But if you guys are ok with it and want to give it a try, I can prepare
> a proper commit.

I think I would like to play with it a little and if we can address the
issues, then yes, I'd love it.

Thanks for your work on this!

/D



More information about the subsurface mailing list