Author Topic: Code AStyle-ification  (Read 4808 times)

Offline gim

  • Zombie Food
  • *
  • Posts: 17
    • View Profile
Code AStyle-ification
« on: March 05, 2013, 06:07:00 PM »
Hi,

I've made test-only astyle-ification branch.
I've suggested it some time ago here: http://whalesdev.com/forums/index.php?topic=264.msg31937#msg31937
and someone else was also suggesting it here:
http://www.cataclysmdda.com/smf/index.php?topic=306.msg2911

You can take a look at (example) results, here:
https://github.com/gim913/Cataclysm-DDA/blob/feature/astyleification/map.cpp

Precise command line, that I've used was
Code: [Select]
AStyle.exe -T8 -H -k1 -j -p -w -Y -W1 -m2 -U -c --style=otbs --lineend=windows

(I've decided to use lineend=windows, cause well, linux editors handle windows'
 newlines much better than windows editors linux's newlines ;))

I haven't sent pull request, as this probably will be quite breaking change for anyone...
a chap go wąż, a wąż go chap

Offline Tase

  • Zombie Food
  • *
  • Posts: 99
    • View Profile
Re: Code AStyle-ification
« Reply #1 on: March 05, 2013, 07:01:33 PM »
I dont imagine this would break anything, just make the code more easy to read :P

Offline GlyphGryph

  • Administrator
  • Survivor
  • *****
  • Posts: 1494
  • Kreaaaaa
    • View Profile
Re: Code AStyle-ification
« Reply #2 on: March 05, 2013, 07:24:28 PM »
Can we just go with actual trustworthy characters and use some variant of number-of-spaces instead? I generally disapprove of tabs for the same reason I disapprove of non-breaking spaces - Invisible stuff that you can't tell what it is without destroying it is not a proper way to write code. It doesn't help that tab behaviour is inconsistent.

Getting multiple spaces to display as a tab or vice versa is trivial, but I think if we're going to standardize, we should standardize with something standard and just say "2 space indentation" or something like that. ... Partially because that's what my tab key already does to my code and I don't want to have to rejigger it. ;)

More seriously, I fully support running running some style standards, but it might be worthwhile to hop in chat in four or five hours when there's a good chance of a couple devs hanging around to go over exactly what those standards should be. And then, after getting input, we can randomly commit to one of the more widely accepted models.

I don't really know what most of your options are short for, so I can't comment beyond saying I dislike tabs and styles that mix broken and attached brackets (of those, 1tbr is definitely the best though), but honestly having it set up like that is a sight better than the current mishmash, so no real complaints. :P
« Last Edit: March 05, 2013, 07:29:04 PM by GlyphGryph »

Offline gim

  • Zombie Food
  • *
  • Posts: 17
    • View Profile
Re: Code AStyle-ification
« Reply #3 on: March 05, 2013, 08:06:16 PM »
Hi, this was to push the discussion in ANY direction, as I don't believe there is anyone who's content with current state...

I'll put some of my arguments here.

Regarding tab vs spaces, I'll repeat stuff from first link:
"tabs for indentation, spaces for formatting"

I'm in a [tab] camp, mainly because you can setup probably any editor, to display it with the width YOU prefer.
(I'm sure about vim, visual studio, code-lite, and I guess code blocks too).

Also in case of some of them, you can actually "display" them (e.g. in vim, you can :match them, in VS you can show them as 'arrows', like in text editor, here's a screenshot http://stackoverflow.com/questions/4065815/how-to-turn-off-showing-whitespace-characters-in-visual-studio-ide)

Getting multiple spaces to display as a tab or vice versa is trivial
I don't think you can do opposite thing easily, that is you have two spaces and display them as four...

and as for tabs, we could actually have pre-commit hooks in git, that would check, that no spaces occur before any printable character...

Regarding brackets, I must say I don't care, as long as it's consistent.

However I'm in favor of ALWAYS using brackets even in case of if with a single statement (this is -j option), that is:
Code: [Select]
if (foo)
{
    bar;
}

and never:
Code: [Select]
if (foo)
    bar;

so in this case 1TBS simply takes less vertical screen-space, but as I've said, I don't care about it, as long as it's consistent.

regarding other options and 'styles', if you have some time, you can take a look here:
http://astyle.sourceforge.net/astyle.html


P.S. I hope no sane person, would ever vote for something like whitesmith or gnu style ;>)
a chap go wąż, a wąż go chap

Offline gim

  • Zombie Food
  • *
  • Posts: 17
    • View Profile
Re: Code AStyle-ification
« Reply #4 on: March 05, 2013, 08:11:41 PM »
I dont imagine this would break anything, just make the code more easy to read :P

well I guess it could easily.

Keep in mind there are at least few people doing different stuff based on CDDA, so if
you make that change in main repo, they would probably have problems merging it to their branches...
a chap go wąż, a wąż go chap

Offline GlyphGryph

  • Administrator
  • Survivor
  • *****
  • Posts: 1494
  • Kreaaaaa
    • View Profile
Re: Code AStyle-ification
« Reply #5 on: March 05, 2013, 08:14:46 PM »
It might be a headache that's worth dealing with, though.

Offline Tase

  • Zombie Food
  • *
  • Posts: 99
    • View Profile
Re: Code AStyle-ification
« Reply #6 on: March 05, 2013, 08:27:26 PM »
I'm for 4-space indentation and allman (--style=allman / --style=ansi / --style=bsd / --style=break / -A1) because 4-space is rather standard and tabs are hidden devils, and allman is more easy to read. I don't mind variations of allman (brackets for one-liners). I'm not really a fan of 1tbs because it makes the start & end of the code blur with the conditionals / elses and strangles the code, I like it when I have space and can easily identify what this if/else/function is doing and where it starts & ends. 1tbs is really a bitch when the identation is broken too, i.e:

Code: [Select]
bool method() {
dosomething()
    if(badcode) {
        dosome();
    doyou();
} else if (something() ) {
    really();
    likethis();
} else
suprise();
}

All I would see in that code is it's closing the brackets 3 times.

Allman example

Code: [Select]
string PrintTheWorld(int index)
{
    if(index >= 0 && index < 10)
        return world[index];
    else
    {
        PrintError("Index out of range for (string)PrintTheWorld(int index)");
        return "";
    }
}


EDIT:

I dont imagine this would break anything, just make the code more easy to read :P

well I guess it could easily.

Keep in mind there are at least few people doing different stuff based on CDDA, so if
you make that change in main repo, they would probably have problems merging it to their branches...

I just merged 1-week old CZS with DDA and if you astyled DDA right now, I'dd merge it without any problems. I really don't think any mod does massive changes to the code, and if they do, they ough to be able to manage their own code, since it's either an addition or a replace.

PS: kevingarnade and DarklingWolf are on IRC right now, join us.
« Last Edit: March 05, 2013, 08:30:49 PM by Tase »

Offline GlyphGryph

  • Administrator
  • Survivor
  • *****
  • Posts: 1494
  • Kreaaaaa
    • View Profile
Re: Code AStyle-ification
« Reply #7 on: March 05, 2013, 09:09:03 PM »
Allman is terrible. You can see brackets closing three times, and you know where they opened because that's the whole damn point of indentation!

Indentation your allman method RUINS. I don't care what bracket it's connected to, I care what /statement/ I'm ending with that close bracket, and your damn broken brackets decouple the relevant logic!

And NOT putting brackets, randomly, for no real reason I have ever been able to tell but confusing the reader, seems bad form in general. Allman is terrrrrrible.

Offline Kevin Granade

  • Administrator
  • Survivor
  • *****
  • Posts: 5588
  • I code dead people.
    • View Profile
Re: Code AStyle-ification
« Reply #8 on: March 05, 2013, 09:38:09 PM »
Line end: unix style
Indent level: 4-space
brackets: Allman, 1tbs, K&R(but why?), or lisp in that order of preference
  All clauses should have brackets, I'm ok-ish with dispensing with brackets for conditionals with an inlined statement like:
Code: [Select]
if( test ) do_stuff();
and possibly other isolated single clause statements, but definitely NOT mixed bracketed and non-bracketed, like:
Code: [Select]
if(test) {
    stuff1;
    stuff2;
} else
    stuff3;
Its like a fun family cookout, except your family is burning in flames while trying to eat you. -secretfire
I'm more excited than a survivor on meth and toast'ems. -Nighthawk
The the giant wasp is slammed through the zombie brute!

Offline GlyphGryph

  • Administrator
  • Survivor
  • *****
  • Posts: 1494
  • Kreaaaaa
    • View Profile
Re: Code AStyle-ification
« Reply #9 on: March 05, 2013, 09:58:27 PM »
"we should do allman, except not" ;)

Can this allstyle thing actually mix and match styles like that? If consistent brackets is important to you, might be best to stick to 1tbs. It's not my favorite style ever, but it does seem to be "choice 2" for everyone.

Offline Tase

  • Zombie Food
  • *
  • Posts: 99
    • View Profile
Re: Code AStyle-ification
« Reply #10 on: March 05, 2013, 10:36:17 PM »
"we should do allman, except not" ;)

Can this allstyle thing actually mix and match styles like that? If consistent brackets is important to you, might be best to stick to 1tbs. It's not my favorite style ever, but it does seem to be "choice 2" for everyone.

Don't be mistaken, I'm saying Allman all the way, there is no broken bracket, indentation isn't broken, there is no randomness, it makes clean, aired and readable code. It's just 1tbs with a newline before an opening bracket. But as for putting single statements in brackets, I don't mind doing that to make someone somewhere happy when reading the code, because like Kevin, that doesn't really bother me. Not as much as allman vs 1tbs.

Discussions 101: Try not to use "your" in arguments, it's not "my" allman, it's not "your" 1tbs, "my" style isn't shitty, "your" style isn't retarded, we're talking about styles, not me or you. Using "your" is the best way to get someone on the defensive and piss them off :)

Anyway, tl;dr I vote for Allman (it's a style, a preference, code won't run faster with or without), single-statement bracketing is a non-issue for me.

Offline GlyphGryph

  • Administrator
  • Survivor
  • *****
  • Posts: 1494
  • Kreaaaaa
    • View Profile
Re: Code AStyle-ification
« Reply #11 on: March 05, 2013, 10:39:52 PM »
Broken brackets is the technical term referring to a bracket on it's own line. So yes, your preferred brackets are broken.

Its not inherently a bad thing, and if Kevin wants allman with enforced brackets on multiline statements, I'm cool with that (the lack of such enforcement is my only real problem with allman aside from the quibble about broken brackets). If we can agree on spaces instead of tabs, I'm happy.

And my decision ultimately doesn't weigh nearly as much as those who are actively contributing to the code.

Offline Tase

  • Zombie Food
  • *
  • Posts: 99
    • View Profile
Re: Code AStyle-ification
« Reply #12 on: March 05, 2013, 10:51:22 PM »
Broken brackets is the technical term referring to a bracket on it's own line. So yes, your preferred brackets are broken.

Its not inherently a bad thing, and if Kevin wants allman with enforced brackets on multiline statements, I'm cool with that (the lack of such enforcement is my only real problem with allman aside from the quibble about broken brackets). If we can agree on spaces instead of tabs, I'm happy.

And my decision ultimately doesn't weigh nearly as much as those who are actively contributing to the code.

Oh I though broken brackets are like
Code: [Select]
{
    }
Like, broken alignment, my bad.

Anyway, I'm for spaces, I don't really like the fact that you can't spot a tab versus a space and the size of the tabs differ depending on the context.

Offline GlyphGryph

  • Administrator
  • Survivor
  • *****
  • Posts: 1494
  • Kreaaaaa
    • View Profile
Re: Code AStyle-ification
« Reply #13 on: March 05, 2013, 10:55:06 PM »
Huh. You may be right, actually... I've always heard them discussed as being broken from the line that determines their context, but I could see it being used to mean staggered as well.

I might have to look that up.

Okay, http://astyle.sourceforge.net/astyle.html seems to define it the way I'm used to, since it says "Allman style formatting/indenting uses broken brackets."

Offline Shades

  • Contributor
  • Zombie Food
  • ****
  • Posts: 61
    • View Profile
Re: Code AStyle-ification
« Reply #14 on: March 06, 2013, 08:40:11 AM »
Personally I prefer tabs over spaces because they let the coder view the code the way they want, everyone uses 1 tab indentation so there is no disagreements and the tab key is generally pressed when indenting regardless of if the editor is set to spaces or tabs.

However I'll take _ANYTHING_* over the 1 space indentation we have right now....


*assuming anything is either a tab or a number of spaces in the set {2, 4}