linleyh / liberation-circuit

Trapped in a hostile computer system, you must make a way out - RTS/coding game
GNU General Public License v3.0
382 stars 40 forks source link

fix indentation #48

Open 229c9cf0 opened 5 years ago

229c9cf0 commented 5 years ago

Indentation is completely broken right now. There's a weird mix of tabs and spaces, indentation depths differs, … It's bad enough that gcc is confused and (wrongly) complains about unguarded statements all over the place:

src/m_input.c:622:6: warning: this ‘else’ clause does not guard... [-Wmisleading-indentation]
src/d_code.c:357:2: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/d_code.c:414:2: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/m_maths.c:232:2: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/m_maths.c:284:2: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/m_maths.c:310:2: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/i_display.c:3536:6: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/i_display.c:4015:7: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/i_display.c:4298:5: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/i_display.c:7282:6: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/i_display.c:13572:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/t_template.c:188:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/v_interp.c:1740:1: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/h_story.c:1279:11: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/h_story.c:1284:11: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/g_method_std.c:273:5: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/e_files.c:596:2: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/v_draw_panel.c:376:1: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/c_fix.c:540:4: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/c_fix.c:940:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/g_game.c:503:2: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/g_world_map_2.c:1675:6: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/t_files.c:195:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/c_compile.c:1305:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/c_compile.c:1367:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/c_compile.c:1425:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/c_compile.c:1464:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/c_compile.c:1536:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/c_compile.c:1631:2: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/c_compile.c:1638:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
src/c_compile.c:1659:4: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]

It's also really hard to read…

Could you fix this?

Alternatively, I could try to set up and run a code formatter on this and make a pull request – in which case, what should the indentation be?

melvinzhang commented 5 years ago

I've tried clang-format on the code base and it works, the trick is to set {SortIncludes: false} as rearranging includes would break compilation. I've checked the generated assembly code on Linux and it is identical after formatting. I can generate some examples of using clang-format with different styles for comparison.

See this gist for c_init.c in different clang-format styles: https://gist.github.com/melvinzhang/def98589d85da808fe6034c6bad6a249

linleyh commented 5 years ago

I won't try to defend any aspect of my terrible code formatting. Anything that makes it less terrible sounds good.

oglinuk commented 2 years ago

I would be happy to go through and clean up the indentation of files, but would like to ask if you prefer tabs or spaces @linleyh?

linleyh commented 2 years ago

Great! Is there a more standard/popular approach to indentation these days? I'm happy either way.

oglinuk commented 2 years ago

I am unsure what others use (other than @melvinzhang suggested clang style), but I try to follow the Linux kernel development coding style. They use 8 character indentation, which they state "Rationale: The whole idea behind indentation is to clearly define where a block of control starts and ends. Especially when you’ve been looking at your screen for 20 straight hours, you’ll find it a lot easier to see how the indentation works if you have large indentations." Would like to get thoughts though.

linleyh commented 2 years ago

If that's how the Linux kernel is indented, it's probably the way to go.

melvinzhang commented 2 years ago

https://github.com/torvalds/linux/blob/master/.clang-format can be used to produce linux kernel style using clang-format. Unfortunately, clang-format is missing support for alignment of defines and initializers, see https://www.kernel.org/doc/html/latest/process/clang-format.html#missing-support

linleyh commented 2 years ago

That should be okay - the code doesn't really have enough defines for that to be a problem, and I don't think it has any designated initialisers.