Code style guide for CLI11

[cbachhuber]

I'm unable to find a code style guide in this project. However, git grep -n guide shows that in App.hpp:62, @henryiii seemed to plan on using the Google C++ Style Guide (GSG).

I suggest using a style guide for CLI11, as it is a prerequisite for consistent code. If you agree, the question would be how this should be implemented. I see three things to consider:

• What to use
  • we could use GSG entirely
  • Only a subset of GSG, which must then be documented
  • Almost everything from GSG, and document only deviations from GSG.
• Where to document: in Contributing.md?
• How to implement the new style guide
  • Existing code does not need to change immediately. New and changed code should comply with GSG. When renaming a member function/variable of a class to comply with e.g. capitalization, all member functions/variables should be renamed to avoid inconsistencies.
  • We could introduce one style guide rule after the other, and with each new rule, directly make the entire code base compliant. I would be happy to take the lead on this.

What do you think?

[henryiii]

A few thoughts:

• Following Google's style guide entirely is not a requirement, a subset or collection of deviations would be fine
• As much as possible should be checked in CI - I'm happy to see the cpplint fork (https://github.com/cpplint/cpplint) supports #pragma once
• Staying fairly close to the current style would be good (but less important than the point above)
• Contributing.md would be a good place unless the guide becomes too large
• I'd be happy to have you take the lead on this. :)

Keep in mind, in the near(ish) future, the will be one large change to the codebase (#338), which will comprise version 2.0, and if any external changes are needed, they ideally would go in then. There will also be custom rules related to the single file generation script. But it will be hard to work in parallel during that change, so I'll probably announce when I start work on that.

The single file script already has custom rules (all CLI11 includes mush have "", all other includes <>), but those will probably become explicit markers like // [cli11:includes] or such.

[cbachhuber]

Thanks for all your input! I will read into cpplint and propose how I would like to approach the code style topic in the next days/weeks.

Also thanks for you hint on #338, I will pause working on code while you're working on that.

[cbachhuber]

Continuing on this topic, I would like to add a license message to the header files in include/CLI and to the respective files in examples to satisfy cpplint's legal/copyright check.

I checked opencv, abseil, EASTL (BSD), and tiny-dnn (BSD) as guidance. From those, I suggest the following license header:

// Copyright (c) 2017-2019, University of Cincinnati, developed by Henry Schreiner under NSF AWARD 1414736 and by the 
// respective contributors
// All rights reserved.
//
// Use of this source code is governed by a BSD-style license that can be found
// in the LICENSE file in the top-level directory of this distribution and at
// github.com/CLIUtils/CLI11.

Compared to the first line of our top-level LICENSE, I added and by the respective authors. Is this okay?

[phlptp]

I would recommend using SPDX identifiers in the headers such as

// Copyright (c) 2017-2019, University of Cincinnati, developed by Henry Schreiner under NSF AWARD 1414736 and by the 
// respective contributors
// All rights reserved.
//
// SPDX-License-Identifier: BSD-3-Clause

or

/* Copyright (c) 2017-2019, University of Cincinnati, developed by Henry Schreiner under NSF AWARD 1414736 and by the respective contributors
 All rights reserved.

 SPDX-License-Identifier: BSD-3-Clause
*/

The single file header should probably include the actual license text on creation.

[cbachhuber]

I'm fine with these solutions - @henryiii, what do you think?

[henryiii]

I like option 1 with spdx. And the license file does get included in the single header version.

[cbachhuber]

I would like to use clang-tidy's readability-identifier-naming to ensure consistent capitalization of code elements.

I saw that quite often, classes and structs are used as 'variables', so I would not constrain their casing. Also, functions sometimes have a leading underscore (mostly, the don't have it, though), which is also not trivial to check with clang-tidy.

In summary, I would for a first shot check the capitalization of variables and function parameters to be lower_case (aka snake_case). This would already require 119 changes (77 in tests) throughout the repo, e.g.

CLI11/include/CLI/Option.hpp:906:34: error: invalid case style for variable 'trueString' [readability-identifier-naming,-warnings-as-errors]
        static const std::string trueString{"true"};
                                 ^~~~~~~~~~
                                 true_string

Do you agree with this initial procedure?