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.

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.

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...

mOctave · 2024-09-17T00:41:14Z

wiki/C++-Style-Guide.md

+
+## Using declarations
+
+Do not put any using declarations in any header file.


I don't have enough C++ experience for this, but does this mean that this:

private: double rotation = 0.; Point position; double zoom = 1.; bool isTargetingFlagship = true; double radius = 15.; const Color *color;

would be illegal?

I'm presuming not, but in that case the style guide should maybe specify that it's namespace declarations that matter.

I don't see a using declaration in that code snippet, and nothing else about it looks like incorrect syntax to me.
This part of the style guide relates to use of the using keyword in header files (outside of class definitions, which should perhaps be added).

Oh, whoops. I somehow missed the word "using." You can ignore this comment, then.

mOctave · 2024-09-17T00:54:32Z

wiki/C++-Style-Guide.md

+    }
+}
+```
+If you are used to putting spaces before the parentheses, rather than trying to adjust your typing you might find it easier to just do a find/replace before committing your code, e.g. replacing "if (" with "if(", etc. You can also grep the svn diff of your changes for "^+.*\ (" to check if you missed anything.


Same issue with blue text here.

Plus, I don't think anyone's "grepped the svn diff" in the last nine? ten? years.

Does this look correct?

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 <[email protected]>

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 <[email protected]>

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.

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 <[email protected]>

mOctave reviewed Sep 17, 2024

View reviewed changes

More in-line code blocks

624c37f

Co-authored-by: mOctave <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

warp-core commented Sep 16, 2024

TomGoodIdea commented Sep 16, 2024

mOctave left a comment

mOctave Sep 17, 2024

warp-core Sep 17, 2024

mOctave Sep 17, 2024

mOctave Sep 17, 2024

warp-core Sep 17, 2024

mOctave Sep 17, 2024

mOctave Sep 17, 2024

mOctave Sep 17, 2024

warp-core Sep 17, 2024

mOctave Sep 17, 2024

mOctave Sep 17, 2024

warp-core Sep 17, 2024

mOctave Sep 17, 2024

mOctave Sep 17, 2024

warp-core Sep 17, 2024

mOctave Sep 17, 2024

mOctave Sep 17, 2024

warp-core Sep 17, 2024

mOctave Sep 17, 2024

warp-core commented Sep 17, 2024

mOctave commented Sep 17, 2024

mOctave left a comment

mOctave Sep 17, 2024

mOctave Sep 17, 2024

mOctave Sep 17, 2024

mOctave Sep 17, 2024

warp-core Sep 17, 2024

warp-core commented Sep 17, 2024

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


		## Using declarations

		Do not put any using declarations in any header file.


		## 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_".

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

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