-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce clang-format #249
Comments
Do you want to change the style of the code or configure it to use the existing style as much as possible? |
I'd start by mimicking the current style, but fix some issues right away, e.g., # from
if(a == b) break;
# to
if (a == b)
{
break;
} We can and should talk about things like where to place the opening bracket (same line vs. separate line) or whether to have a space between The idea is to apply these changes only to code modified within the scope of commits anyway, so even breaking code style changes (e.g., changing the spacing) will not bloat the amount of lines changed significantly. I do not intend to reformat the entire code base, since the amount of lines to be reviewed would be way too large. |
I think most tools are fine to use as long as exceptions are allowed where a human thinks it's better to do it differently. Like if you have many small if-statements in a row I think it can be more readable to make them one line each, like: if (s == "a") do_a();
else if (s == "b") do_b();
else if (s == "c") do_c();
// ...
else print_error("Unknown action"); Another example; I usually put the opening brace on the same line in other projects, but if the head part gets so long that it's split into multiple lines, I think it's much more readable to put it on the next line: for (int i = 0; i < 5; i++) {
printf("Hello world\n");
}
// Looks bad
for (const struct ListNode *item = my_list.first;
item != NULL;
item = item->next) {
printf("Hello world\n");
}
// Better
for (const struct ListNode *item = my_list.first;
item != NULL;
item = item->next)
{
printf("Hello world\n");
} Or if your style is to not put spaces after This all means that it's good to be careful with making automatic formatting too automatic or mandatory. |
clang-format would be cool indeed, but before jumping into actually writing rules for it, the current codestyle wiki entry must be updated for cases like the ones mentioned above. For a tasklist:
FWIW, I actually prefer using Linux's or FreeBSD's codestyle because they are both very clearly defined and easy to use/understand, but ofc to each his own. |
I think big PRs like #248 show that a code formatter and code format linter would greatly help improve the code quality within this repository. Since the initial fork over two years ago, I have worked with clang-format in a few projects, and I like how it works. It can auto-format only changed areas, sort headers, automatically lint changes (e.g., in PRs), easily be used in a pre-commit hook, etc.
In my opinion, we should codify the future code style for the project in a clang-format config included in this repository, and enforce its use during PRs using a separate GitHub action. This will save time and increase the readability quite a bit, thus saving developer time.
What do you think?
The text was updated successfully, but these errors were encountered: