Skip to content
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

Add .clang-format file #522

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add .clang-format file #522

wants to merge 5 commits into from

Conversation

insunaa
Copy link
Contributor

@insunaa insunaa commented Mar 23, 2022

🍰 Pullrequest

This PR adds a .clang-format file. IDEs which integrate clang-format can use this to automatically format their code to CMaNGOS code-style as defined by https://github.com/cmangos/issues/wiki/Coding-Standards

Todo / Checklist

  • Make sure that code-style is completely coherent

@insunaa
Copy link
Contributor Author

insunaa commented Mar 23, 2022

This format file uses the line-length of 80 prescribed by the style-guide, but imo this creates pretty cramped looking code. Imo a higher value, like 120 should be used

@Fabi
Copy link

Fabi commented Mar 23, 2022

This format file uses the line-length of 80 prescribed by the style-guide, but imo this creates pretty cramped looking code. Imo a higher value, like 120 should be used

more like 180, 220 or 240 imo

@insunaa
Copy link
Contributor Author

insunaa commented Mar 23, 2022

My editor with a pretty small font is ~ 230 columns wide, I'd be on board with 180, but I think more than that would be a bit overkill and then there might as well be no line-length restriction

@insunaa
Copy link
Contributor Author

insunaa commented Mar 23, 2022

https://gist.github.com/insunaa/e6d9f505877f314e9c5c329dbec3c536 this Gist has diff files between the original Creature.cpp and Creature.cpp treated with clang-format at 80, 120 and 180 line-length

@killerwife
Copy link
Contributor

My personal rule of thumb is 120 but i have made exceptions where I have made up to 180 cos of code simplicity over readability. So I would go with 180, because we should already add line breaks for sake of readability.

@killerwife
Copy link
Contributor

And also we have a verticality problem in many files, one of the reasons i dont like stacking constructor member init one per line. Especially in scripts.

@insunaa
Copy link
Contributor Author

insunaa commented Jul 12, 2022

There has been no further discussion happening. Do we want a standardized format file or not? If so I would request we merge this PR. If not let me know so I can clean the cluttered PR page.

@killerwife
Copy link
Contributor

@cyberium please check it and merge it. @Warlockbugs maybe speak your opinion here.

@Warlockbugs
Copy link
Member

Hopefully, this wont cause enforced formatting on entire file on any change made to that file. That would make submitting and merging PRs miserable, as well complicate git history quite a bit.

@insunaa
Copy link
Contributor Author

insunaa commented Sep 12, 2022

It does not enforce formatting, but if you select the whole file and use the auto-formatting feature it will obviously format the whole file. Generally you're supposed to select whatever you want to format and then use the auto-format feature for only that area. I don't know if some IDEs enforce formatting the entire file, tho.

@insunaa
Copy link
Contributor Author

insunaa commented Apr 1, 2024

I don't know if the syntax is even still valid, but the discussion about this addition should not just stay dead, imo.
Maybe we should also add a .clang-tidy file, which allows automatic highlighting of various problems, such as going against the Cpp core guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants