Copy the C++ style guide into the wiki #47

warp-core · 2024-09-16T21:53:42Z

New content

Summary

It has been suggested on occasion that the C++ style guide (available on the website) should be inside the wiki.
This PR copies and reformats that content into a markdown file within the wiki.

TomGoodIdea · 2024-09-16T22:01:41Z

I'd prefer the website redirecting here, or having some method to sync automatically (but there's format difference). Hosting it on both places makes it harder to modify.

mOctave

Quite a few comments on this.

Most of my comments have to do with changes to the styleguide itself (or changes to our current practices), so may be best dealt with in a separate PR. The biggest ones here stem from the fact that this style guide hasn't been updated in a decade or so, and I think Endless Sky switched which platform it was hosted on two or three times in that period?

The other two things I've noticed is that, since markdown has this, we should probably use it instead of MZ's "double-quoted code," and that I think code blocks might need an extra line after their preceding paragraph in order to work possibly.

Also, we might want to make edits now that this isn't a list where you click to expand points. It seems to mostly work as-is, though.

Finally, I would very much support moving this over to only be on the wiki. I think the fact that this is so very outdated emphasizes how little people actually use the website, especially when writing code (or making sweeping changes to little things like the version of C++ ES uses!)

mOctave · 2024-09-17T00:35:52Z

wiki/C++-Style-Guide.md

+this program. If not, see <https://www.gnu.org/licenses/>.
+*/
+```
+If you make substantial additions or changes to a file, please add your name to the copyright list.


Haha, very funny MZ. This should probably either be removed, or we should go back and actually start doing copyright headers properly in the first place.

Can you elaborate on what it would mean to do them properly?
As I understand it, the current approach is indeed for people who make substantial changes to a file, or create new code in a new file, to add their name to the copyright at the beginning, though this is perhaps more common in relation to data files than it is to the source code.

As far as I'm aware, people's names only get put in the copyright header when they're the one who makes it, for ES. At least, that's the rules I've been playing by. I think, according to the GPL license, every single contributor to the file should have their name in the header. We definitely aren't doing that.

I know for certain that people who have made significant additions to data files have been added to the copyright header.
I don't know about the C++ files, but it's possible that no one has made sufficient changes to a single file that they did not create to get to that point, at least not in one go, or they just didn't want to put their name there, or I'm just forgetting instances where it has happened.
I can't see where the GPL says that. It certainly references "authors". I don't think every contribution to a file constitutes "authorship".

mOctave · 2024-09-17T00:36:44Z

wiki/C++-Style-Guide.md

+
+Within a .h file, the #includes should be broken into "paragraphs" each separated by a single blank line:
+1. Classes that this file's class inherits from.
+2. Other files in the "endless-sky" code base.


Any reason why this is "endless-sky", rather than Endless Sky or even just endless-sky?

I don't know why it was originally written that way. As mentioned, this PR just copies the content of the existing style guide and reformats it from xml.
Do you think there is something wrong with the way it is right now?

Not particularly anything wrong, this could just be something to change in a follow-up PR.

Or, it could be formatted as

`endless-sky`

Suggested change

2. Other files in the "endless-sky" code base.

2. Other files in the `endless-sky` code base.

How would this change the appearance and why do you think it would be an improvement?

mOctave · 2024-09-17T00:37:22Z

wiki/C++-Style-Guide.md

+class Angle;
+class Sprite;
+```
+It is helpful but not required to alphabetize the lines within each "paragraph" of #includes.


Maybe we should require this? Is there any place where we don't do this?

In practice, we do require it. This can be changed.

mOctave · 2024-09-17T00:37:57Z

wiki/C++-Style-Guide.md

+## Order of #includes in a .cpp file
+
+Within a .cpp file, the #includes should be broken into "paragraphs" each separated by a single blank line:
+1. This file's corresponding .h.
+2. Other files in the "endless-sky" code base.
+3. Non-standard third-party libraries.
+4. Standard libraries.
+
+For example, a .cpp file might contain the following #includes:
+```c++
+#include "MyClass.h"
+
+#include "Angle.h"
+#include "Sprite.h"
+
+#include &lt;SDL/SDL.h&gt;
+
+#include &lt;set&gt;
+```
+It is helpful but not required to alphabetize the lines within each "paragraph" of #includes.
+


I can't help but feel like this could be combined with the section above...

It could, but would that be an improvement?

wiki/C++-Style-Guide.md

mOctave · 2024-09-17T00:56:43Z

wiki/C++-Style-Guide.md

+}
+```
+
+## Function definitions


Blue text again

mOctave · 2024-09-17T00:59:08Z

wiki/C++-Style-Guide.md

+
+## No compiler warnings
+
+Code should compile with -Wall without any warnings.


I already get compilation warnings every time I build for MacOS. Are we still following this guideline, or can we safely ignore specific compilation warnings?

As far as I know, no warnings are produced in the CI/CD MacOS builds.
Building using supported tools should not produce warnings with -Wall.
Consider opening an issue providing more details if this is not consistent with your experience.

mOctave · 2024-09-17T01:00:38Z

wiki/C++-Style-Guide.md

+
+
+
+# C++11


We should maybe have a section for later versions of C++, too?

wiki/C++-Style-Guide.md

warp-core · 2024-09-17T01:47:25Z

In general, I'm not inclined to change the content of the style guide in this PR, only the formatting.
You can already propose changes to it in the repository for the website at https://github.com/endless-sky/endless-sky.github.io.

Co-authored-by: mOctave <73318970+mOctave@users.noreply.github.com>

mOctave · 2024-09-17T02:47:42Z

Yeah, that makes sense. I still would suggest taking it off the website, though, since the website in general is the last place anyone looks for anything.

Also, I'm pretty sure I didn't catch all the quoted text that should be changed. I can go through it again and find some more if you'd like.

mOctave

I think this is all of them, now.

wiki/C++-Style-Guide.md

mOctave · 2024-09-17T03:05:49Z

wiki/C++-Style-Guide.md

+
+## File extensions
+
+All C++ files should use ".h" and ".cpp" for their extensions.


Suggested change

All C++ files should use ".h" and ".cpp" for their extensions.

All C++ files should use `.h` and `.cpp` for their extensions.

mOctave · 2024-09-17T03:06:10Z

wiki/C++-Style-Guide.md

+
+## One class per file
+
+Each class "MyClass" should have its own .h and .cpp files, and they should be named "MyClass.h" and "MyClass.cpp".


Suggested change

Each class "MyClass" should have its own .h and .cpp files, and they should be named "MyClass.h" and "MyClass.cpp".

Each class `MyClass` should have its own .h and .cpp files, and they should be named `MyClass.h` and `MyClass.cpp`.

mOctave · 2024-09-17T03:06:24Z

wiki/C++-Style-Guide.md

+
+## Names for files with no classes
+
+If a file does not contain any classes (such as "main.cpp" or "shift.h"), its name should be lower-case.


Suggested change

If a file does not contain any classes (such as "main.cpp" or "shift.h"), its name should be lower-case.

If a file does not contain any classes (such as `main.cpp` or `shift.h`), its name should be lower-case.

mOctave · 2024-09-17T03:06:33Z

wiki/C++-Style-Guide.md

+
+## Header file guards
+
+All header files should have #define guards, and they should be in the format "#ifndef MY_CLASS_H_".


Suggested change

All header files should have #define guards, and they should be in the format "#ifndef MY_CLASS_H_".

All header files should have #define guards, and they should be in the format `#ifndef MY_CLASS_H_`.

I need to update lines about header guards anyway, so this will be removed.

wiki/C++-Style-Guide.md

Co-authored-by: mOctave <73318970+mOctave@users.noreply.github.com>

warp-core · 2024-09-17T11:19:59Z

I'm not so sure about putting things that aren't code (file names, for example) in in-line code blocks instead of quotes.

TomGoodIdea

Let's just finish the migration for now. Any updates to the guide contents themselves that aren't already present in the old version should be properly discussed in their own separate PRs after the migration is complete.

TomGoodIdea · 2025-03-22T10:08:08Z

wiki/C++-Style-Guide.md

+All header files should have #define guards, and they should be in the format "#ifndef MY_CLASS_H_".
+The C++ standard technically does not allow source code files to define macros that start with an underscore or contain a double underscore.


endless-sky/endless-sky.github.io@a4afddd

Suggested change

All header files should have #define guards, and they should be in the format "#ifndef MY_CLASS_H_".

The C++ standard technically does not allow source code files to define macros that start with an underscore or contain a double underscore.

All header files should have #pragma once guards.

The C++ standard technically does not allow #pragma once, but all major compilers support it. It is also much easier to use than regular header guards.

TomGoodIdea · 2025-03-22T10:08:21Z

wiki/C++-Style-Guide.md

+8. private variables
+Anything not listed here (e.g. public non-constant variables, protected variables, etc.) should not be used.
+Here is an example class definition, with extra comments added to point out aspects of the formatting:


Suggested change

8. private variables

Anything not listed here (e.g. public non-constant variables, protected variables, etc.) should not be used.

Here is an example class definition, with extra comments added to point out aspects of the formatting:

8. private variables

Anything not listed here (e.g. public non-constant variables, protected variables, etc.) should not be used.

Here is an example class definition, with extra comments added to point out aspects of the formatting:

TomGoodIdea · 2025-03-22T10:08:41Z

wiki/C++-Style-Guide.md

+
+## Function names
+
+Function names should be CapitalizedCamelCase


Suggested change

Function names should be CapitalizedCamelCase

Function names should be CapitalizedCamelCase.

TomGoodIdea · 2025-03-22T10:08:59Z

wiki/C++-Style-Guide.md

+
+## Single-line loop bodies
+
+You do not need to use braces if a loop or conditional body is a single expression.


endless-sky/endless-sky.github.io@c541362

Suggested change

You do not need to use braces if a loop or conditional body is a single expression.

You should not use braces if a loop or conditional body is a single expression.

TomGoodIdea · 2025-03-22T10:09:01Z

wiki/C++-Style-Guide.md

+if(theThing != done)
+    DoTheThing();
+```
+You can also forgo the braces for something like this:


Suggested change

You can also forgo the braces for something like this:

You should also forgo the braces for something like this:

TomGoodIdea · 2025-03-22T10:09:02Z

wiki/C++-Style-Guide.md

+    if(vec[i])
+        sum += i;
+```
+And, you can also do this:


Suggested change

And, you can also do this:

And, you should also do this:

warp-core added 4 commits September 16, 2024 22:47

This is probably out of date by now

907ed26

Include a link to the page on the website

29f8603

Update sidebar link

6b05fd8

Update homepage link

c1dca1e

TomGoodIdea added the standalone A PR that doesn't require an endless-sky PR to be merged. label Sep 16, 2024

mOctave reviewed Sep 17, 2024

View reviewed changes

Replace html character references with the actual characters

8455b16

Replace quotes with backticks for markdown in-line code blocks.

9d09bda

Co-authored-by: mOctave <73318970+mOctave@users.noreply.github.com>

mOctave reviewed Sep 17, 2024

View reviewed changes

More in-line code blocks

624c37f

Co-authored-by: mOctave <73318970+mOctave@users.noreply.github.com>

TomGoodIdea reviewed Mar 22, 2025

View reviewed changes

	2. Other files in the "endless-sky" code base.
	2. Other files in the `endless-sky` code base.


		## No compiler warnings

		Code should compile with -Wall without any warnings.


		## File extensions

		All C++ files should use ".h" and ".cpp" for their extensions.

	All C++ files should use ".h" and ".cpp" for their extensions.
	All C++ files should use `.h` and `.cpp` for their extensions.


		## One class per file

		Each class "MyClass" should have its own .h and .cpp files, and they should be named "MyClass.h" and "MyClass.cpp".

	Each class "MyClass" should have its own .h and .cpp files, and they should be named "MyClass.h" and "MyClass.cpp".
	Each class `MyClass` should have its own .h and .cpp files, and they should be named `MyClass.h` and `MyClass.cpp`.


		## Names for files with no classes

		If a file does not contain any classes (such as "main.cpp" or "shift.h"), its name should be lower-case.


		## Header file guards

		All header files should have #define guards, and they should be in the format "#ifndef MY_CLASS_H_".

		All header files should have #define guards, and they should be in the format "#ifndef MY_CLASS_H_".
		The C++ standard technically does not allow source code files to define macros that start with an underscore or contain a double underscore.


		## Function names

		Function names should be CapitalizedCamelCase


		## Single-line loop bodies

		You do not need to use braces if a loop or conditional body is a single expression.

	You do not need to use braces if a loop or conditional body is a single expression.
	You should not use braces if a loop or conditional body is a single expression.

	You can also forgo the braces for something like this:
	You should also forgo the braces for something like this:

Copy the C++ style guide into the wiki #47

Are you sure you want to change the base?

Copy the C++ style guide into the wiki #47

Conversation

warp-core commented Sep 16, 2024

Summary

TomGoodIdea commented Sep 16, 2024

mOctave left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

warp-core commented Sep 17, 2024

mOctave commented Sep 17, 2024

mOctave left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

warp-core commented Sep 17, 2024

TomGoodIdea left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment