Update our style guide #5500
Update our style guide #5500
Conversation
|
Before we merge this I'd like to invite the community to weigh in. I know there are some 2 space fans out there but the majority of people seem to prefer 4 space indents, and a lot of our code is already indented that way. |
|
4 spaces? noooooooooo! :) Ah, whatever. I'm happy enough just letting clang-format deal with it. |
|
I'm going to need a bigger screen⸮ |
|
4 space master race wow it's me vs 3 others so far |
|
Could you summarize what the reasoning behind the switch from 2 to 4 is? I think most of the codebase is currently indented using 2 spaces, but I'll try to get some numbers to back that up. |
|
I used to be a 2 spacer, but do 4 now, as it’s visually easier to distinguish, especially since I like to use a relatively narrow font. @skullydazed Can we also get some guidance on indentation of #ifdefs/#ifs? I’ve ran into different styles for that lately. |
|
4 spaces FTW for visual clarity and easy for beginners because this is also used by python |
|
Don't reference other things as an authority, as there's a strong example for every possible sensible combination you're after (also, Python happily uses whatever indentation you want, as long as it's consistent in the block.) It's more important that whatever is chosen minimises the amount of work required. Right now, most of the codebase uses 4 spaces, and it would have to be entirely reformatted to change that. The other concern is being able to automate most of the reformatting. It's far easier to maintain a style when nobody has to do it manually. But clang-format isn't perfectly configurable, so either you fight it constantly, or you get it as close as possible and then accept that as the baseline. |
|
A quick and dirty scan over the
There's no clear winner, but 2 spaces is noticeably ahead (possibly because that's been the recommended size for a while). I used this package for detecting indentation. Submodule files were ignored, and I did the scan on a fresh clone of the project. The method I used isn't perfect, and there are more files that could've been ignored (e.g. the third-party files in The “ambiguous spaces” category is mostly header files that have no indentation, but were detected as 1 space due to a leading space being used for alignment in block comments. 1144 out of the 1203 files are like this. The rest are oddball files that were detected as 12 spaces, 7 spaces etc. With the objective part out of the way, I personally prefer 4 spaces for most projects. However, I also think that 2 spaces works well in QMK (keymaps are one example), and that a lot of files would need to be updated over time if the recommended indentation size were to change. So it might be wise to think this over a bit more.
This is incorrect, as you can see in the table above. A lot of reformatting will need to be done either way if we are to eventually achieve consistency. |
|
2 spaces has made my eyes bleed. I welcome the 4 space overlords. |
|
Per @nooges' comment and after some discussion on discord I've updated the |
| * When deciding how (or if) to indent directives keep these points in mind: | ||
| * Readability is more important than consistency. | ||
| * Follow the file's existing style. If the file is mixed follow the style that makes sense for the section you are modifying. | ||
| * When choosing to indent you can follow the indention level of the surrounding C code, or preprocessor directives can have their own indent level. Choose the style that best communicates the intent of your code. |
There was a problem hiding this comment.
Here's the suggestion from Discord:
| * When choosing to indent you can follow the indention level of the surrounding C code, or preprocessor directives can have their own indent level. Choose the style that best communicates the intent of your code. | |
| * When choosing to indent you can follow the indention level of the surrounding C code, or preprocessor directives can have their own indent level. Choose the style that best communicates the intent of your code. | |
| * In either case, preprocessor indentation should not introduce additional levels of indentation to actual code. Otherwise, you can end up with code that's very hard to read (see [here](https://gist.github.com/vomindoraan/7934814ef7a6301f4ef1f462df3386df) for an example). |
I welcome any suggestions to my suggestion 😄
* Update our style guide * Clarify muiltple condition ifs * update the ifdef section
- Fixes the vertically-offset key issue. - Layout macros and keymap.c layers actually look like the keyboards now. :) - Code output now uses four-space indent - because qmk/qmk_firmware#5500 (I really should go through this and pull out the dead code. Feel like there's a lot.)
* Update our style guide * Clarify muiltple condition ifs * update the ifdef section
* Update our style guide * Clarify muiltple condition ifs * update the ifdef section
Description
Update our style guide to reflect a couple recent things that have been coming up.
#ifdefvs#if defined()Types of Changes
Issues Fixed or Closed by This PR
Checklist