AStyle - time to establish coding style rules

Game Talk The Lab
1

I have git’s pre-commit hook to run AStyle working on windows (and am told it works on linux but not Mac[sup]*[/sup]);
so now we need to decide what coding style that will enforce.

If we do agree to use this- the entire repository will be styled with this tool[sup]edit:see note[/sup], then all future commits will be automatically styled using this tool. [shadow=red,left].gitattributes[/shadow] will also be changed to reflect the prefered indentation and line ending scheme.

Stypling will be applied on commit to specific file types, or even specific files as dictated by replies to this thread.

Rather than use a poll, I would like to have actual replies influence what happens as we work through the styling options.

The fastest way I can figure is to ask people to put together the set of options they would like, and what commonality between them becomes what goes into the rule set.

The important thing is that we get something smarter than mixed single-space tab 4-space 8-space indentation with irregular windows/linux endings.

ASTYLE_PARAMETERS_CPP="--style=1tbs \ --indent=spaces=4 \ --indent-classes \ --indent-switches \ --indent-namespaces \ --indent-preprocessor \ --indent-labels \ --indent-col1-comments \ --unpad-paren \ --keep-one-line-blocks \ --keep-one-line-statements \ --lineend=linux \ --suffix=none" #ASTYLE_PARAMETERS_H=... #ASTYLE_PARAMETERS_JSON=...

My inclination is to see 4-spaces for cpp, and 2-space for json; with linux endings through out.

see also:
[sub]zaphire / Monocle-Engine [/sub]pull#108 : AStyle usage for consistent code style
[sub]Applied Informatics[/sub]C++ Coding Style Guide
http://astyle.sourceforge.net/
https://www.archlinux.org/packages/community/x86_64/astyle/

edit: as per IRC discussion, one file at a time, over a period of many days.

AStyle_doc_2.03.zip (49.6 KB)

2

Yeah, I was going to say that 4-space is good for .cpp, 2-space for JSON.

Also, is this something that has to be configured on the devs’ local machines? Because if not, then I wouldn’t necessarily say it’s “automatic” (it’s hard enough sometimes getting clean commits from new contributors, and we tend to err on the side of being newbie-friendly).

3

You raise a valid point. Perhaps I should add (.bat and .sh) files that will make the appropriate changes to the [tt].git/hooks[/tt] file, instead of asking them to copy it.

4

That’s something that seems to make sense - I don’t necessarily expect newbies to know how to edit Git configuration files, but saying “we’d really appreciate it if you ran this file before committing” is workable, I think.

5

^- -^ makes sense; I’ll focus on the [tt].sh[/tt] file.

So the deployment plan is to establish the one coding style, then apply it one file at a time. As each file is styled, it is added to the list of files that will be automatically styled when the script is run.

And it is decreed that the standard for cpp is 4-spaces , no tabs ever.

6

Yeah, if we have to touch an entire file, I’d say only apply it once at least 80% of the file is whitespace-conformant with our chosen style (I’m less concerned about a mass conversion between, say, Allman braces and 1TBS, since that’s MUCH fewer lines that’ll be touched).

There was also some mention on IRC of possibly changing the bracketing style again, away from Allman. I know that GlyphGryph is strongly in favor of 1TBS over Allman, I have a slight preference for 1TBS, and Kevin seems on the fence. Dunno about other contributors, though.

7

Java style formatting/indenting uses attached brackets.

I am a lisp/java programmer; so I’d use ‘java’ if not ‘1tbs’ style- since every programmer I’ve had to work with have thrown fits and mice when I hand them lisp formatted code.

(from the AStyle manual)

[ul][li][tt]–style=allman / --style=ansi / --style=bsd / --style=break / -A1[/tt]
Allman style formatting/indenting uses broken brackets.[/li]
[li][tt]–style=1tbs / --style=otbs / -A10[/tt]
“One True Brace Style” formatting/indenting uses linux brackets and adds brackets to unbracketed one line conditional statements. In the following example brackets have been added to the “[tt]return 0;[/tt]” statement.
The option --add-one-line-brackets can also be used with this style.[/li]
[li][tt]–style=java / --style=attach / -A2[/tt]
Java style formatting/indenting uses attached brackets.[/li]
[li][tt]–style=lisp / --style=python / -A12[/tt]
Lisp style formatting/indenting uses attached brackets, opening brackets are attached at the end of the statement. The closing bracket is attached to the last line in the block. The style implies keep-one-line-statements but NOT keep-one-line-blocks. This style does not support one-line brackets. If add-one-line-brackets is used they will be added as multiple-line brackets.[/li][/ul]

[table][tr]
[td][b][tt]–style=allman[/td]
[td][b][tt]–style=1tbs[/td]
[td][b][tt]–style=java[/td]
[td][b][tt]–style=lisp[/td]
[/tr][tr]
[td]

[tt]int Foo(bool isBar)[/tt]
{
[tt] if (isBar)
[/tt]{
[tt] bar();
return 1;
[/tt]}
[tt] else
return 0;[/tt]
}
[/td]
[td]
[tt]int Foo(bool isBar)[/tt]
{[tt]
if (isFoo) [/tt]{
[tt] bar();
return 1;
[/tt]}[tt] else [/tt]{
[tt] return 0;
[/tt]}
}
[/td]
[td]
[tt]int Foo(bool isBar) [/tt]{[tt]
if (isBar) [/tt]{[tt]
bar();
return 1;
[/tt]}[tt] else
return 0;[/tt]
}
[/td]
[td]
[tt]int Foo(bool isBar) [/tt]{[tt]
if (isBar) [/tt]{[tt]
bar()
return 1; [/tt]}[tt]
else
return 0; [/tt]}
[/td]
[/tr][/table]

8

Hi, I’m KA101, and I’m a Git newbie. I haven’t enough of a clue how to implement the existing pull-protocols. Having a “hit this button” for folks using the web/shell rather than the command line would be eminently sensible, as would a step-by-step post on how to config one’s git. (I can’t find the “delete my repo and start over” button.)

9

Wow, I never actually thought changing the bracketing style again would become an option, and I’d finally gotten used to Allman, but if it is, yeah I’ll come down in favour of a change to 1tbs.

4space c++/2space json is fine with me. Though we might as well go 2/2? I don’t actually care, though, as long as it’s consistent.

I think the bar for auto-conversion should be much lower though, no more than 50% of a file required. I honestly think the confusion and difficulty of tracing meaningful history of “change as you go” is much worse than just outright full-file conversion. It’s much easier to figure out what was legitimately an important part of future pull requests when I know every line that’s changed is important. I’d much rather shove all the “unimportant” changes for a file into one easy-to-ignore commit. For the less commonly edited files, I’d say there’s no reason not to convert them over immediately, since they’re never likely to reach the 50% threshold and I think conformance to project standards across the entire application is valuable.

10

I’d just like to say that I’ve developed a strong dislike of AStyle’s Java-style brackets, at least when combined with single-space indents. Specifically, mixing bracketed and bracket-less if/else clauses. I greatly prefer when they’re all uniformly bracketed or unbracketed.

11

I realize that I don’t have much say, since I haven’t yet contributed much, but I’ll throw my two farthings onto the table.

Concerning braces, I must side with Joel Spolsky, to wit, always use them. He’s written definitively on the subject, which I won’t include here for the sake of brevity, but it’s worth a read.

Concerning spacing, I’m in favor of 4-spaces in source files, and 2-spaces in json files.

If you’ve read this far, then thank you for your consideration! :smiley:

Another worthwhile read is The Law of Leaky Abstractions, which touches on many topics including driving in the rain, which I found fitting for C:DDA. :wink:

12

IMO 4 is much more easily readable for actual code then 2 is.

13

For cpp/h - 1tbs,4-spaces,linux line endings.
For json - ???,2-spaces,linux line endings.

edit:
I’ve gotten around to comparing the build outputs; and have determinded that after AStyle these files do not compile the same:

[ul][li]artifact.cpp[/li]
[li]disease.cpp[/li]
[li]game.cpp[/li]
[li]iexamine.cpp[/li]
[li]inventory_ui.cpp[/li]
[li]iuse.cpp[/li]
[li]map.cpp[/li]
[li]mapgen.cpp - broken wrapped strings[/li]
[li]npcmove.cpp[/li]
[li]vehicle.cpp[/li][/ul]

14

Anyone focused on JSON want to say something?

15

Json has json style, last I checked. We can probably just keep it that way.

[
{
blah: [ “one”, “two”, “three”],
hoo: “hah”
},
{
checking: [ “one, two”]
}
]

And whatnot.

16

IMO 4 is much more easily readable for actual code then 2 is.[/quote]

I strongly disagree. 4 spaces explodes the code to near unreadability.

2 spaces is the only logical choice, especially if we’re using that for the JSON.

17

[quote=“Williham, post:16, topic:1579”]I strongly disagree. 4 spaces explodes the code to near unreadability.[/quote]Heh. I find it funny that you should say that – to me, 2 spaces compresses the code to near unreadability. But… I tend to have very wide terminals.

Personally, I never understood why people hate the Tab character so much (for leading indentation). It’s designed to be a special character whose meaning is up to the individual looking at it; I can have my tab stops at 4 characters, you can have yours at 2 characters, and we’d both be perfectly happy.

18

The important thing is that we get something smarter than mixed single-space tab 4-space 8-space indentation with irregular windows/linux endings.

ASTYLE_PARAMETERS_CPP="--style=1tbs \ --indent=spaces=4 \ --indent-classes \ --indent-switches \ --indent-namespaces \ --indent-preprocessor \ --indent-labels \ --indent-col1-comments \ --unpad-paren \ --keep-one-line-blocks \ --keep-one-line-statements \ --lineend=linux \ --suffix=none" #ASTYLE_PARAMETERS_H=... #ASTYLE_PARAMETERS_JSON=...

I’ve been avoiding this because I knew I’d have to go through all the astyle options and spend some time considering them, here are my preferences for cpp/h. I’ll need to take a closer look at json, though 2-space is probably sensible.

–style=1tbs // Yes, this is not what we’re currently using, Allman was a compromise IIRC
–indent-spaces=4 //tellingly, leaving this off defaults to 4-space indention :3
–align-pointer=name // I’m more used to =name, I don’t think =type would be a huge problem, but I do want to pick one or the other.
–max-code-length=100
–break-after-logical

I’d be very curious to hear why anyone would not want these (indention of switch statements is weird, and I’d not be surprised about disagreement of them in particular).
–indent-all-the-things
–pad-oper
–add-brackets

There is contention about this, but AFAIK there is total consensus amoung the main devs about it, so arguing about it is pretty poitless.
–convert-tabs

I think paren padding can and should be left to the author, for emphasis. Also if there is enough paren nesting to make it a significant issue, it should probably be broken up anyway.

Side note, I’m quite amused that the sample formatting for style=linux violates linux kernel coding standard (linux uses --add-brackets)

19

I have a moderately/reasonably strong preference for --align-pointer=type (it seems more semantically correct to me), but it’s not something I’ll raise a fuss over.

I don’t have enough opinion on --break-after-logical to bother weighing in.

Agreed that parenthesis padding should be left to humans, since what I consider to be “correct” depends on context (unpadded is my default, but there are some cases where I think the padding is worth breaking style).

I actually prefer tabs to spaces, but not nearly as much as I prefer one or the other to mixed tabs and spaces. So yeah, --convert-tabs is good.

Strongly in favor of --add-brackets, especially with 1TBS. I particularly dislike cases where brackets are used inconsistently (e.g., on the middle else if, but not on the initial if or the final else).