[clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[MyDeveloperDay]

Summary

a change D67541: [ClangFormat] Future-proof Standard option, allow floating or pinning to arbitrary lang version cause LanguageStandard to now be subtly different from all other clang-format options, in that the Enum value (less the prefix) is not always allowed as valid as the configuration option.

This caused the ClangFormatStyleOptions.rst and the Format.h to diverge so that the ClangFormatStyleOptions.rst could no longer be generated from the Format.h using dump_format_stlye.py

This fix tried to remedy that:

1. by allowing an additional comment (in Format.h) after the enum to be used as the in configuration ( XXXX ) text, and changing the dump_format_style.py to support that.

This makes the following code:

enum {
...
LS_Cpp03, // c++03
LS_Cpp11, // c++11
...
};

would render as:

* ``LS_Cpp03`` (in configuration: ``c++03``)
* ``LS_Cpp11`` (in configuration: ``c++11``)

And we also move the deprecated alias into the text of the enum (otherwise it won't be added at the end as an option)

This patch includes a couple of other whitespace changes which help bring Format.h and ClangFormatStyleOptions.rst almost back into line and regeneratable... (there is still one more)

[MyDeveloperDay]

clang/include/clang/Format/Format.h

1971

my assumption here was that the text as was in the .rst was correct, hence why I'm reflecting that text back into Format.h

1986–1987

Again focus is on the rst being the desired text and getting them to be aligned. @sammccall please correct me if my assumption is incorrect

[MyDeveloperDay]

For help for the reviewers...here is the before and after

Before:

After:

[mitchell-stellar]

clang/docs/tools/dump_format_style.py

175

This seems quite flimsy to me, as it depends on an undocumented comment style. It is true that if the file(s) in question are properly clang-formatted, then this would probably not fail, but it does not appear to be a very robust solution.

[MyDeveloperDay]

clang/docs/tools/dump_format_style.py

175

I'd tend to agree, but this whole dump_format_style.py is flimsy.. take a look at this review D31574: [clang-format] update documentation

When you added this line, you forgot the third /

// Different ways to wrap braces after control statements.

Also, the extra empty line in the LanguageStandard both caused the whole python file to fail with an exception.

Do you have a suggestion for something better? (which doesn't leave the Format.h looking too odd)

[mitchell-stellar]

clang/docs/tools/dump_format_style.py

175

I would go back to the /// c++03: Parse and format as C++03. style. /// is a Doxygen comment, and I think documentation should be generated solely from Doxygen comments, even if it requires a bit of post-processing. (The extra / needed after // in the ticket you mentioned is justified.)

[MyDeveloperDay]

clang/docs/tools/dump_format_style.py

175

The Doxygen documentation is used for source-level documentation, this is user-level documentation which the restructured text output .rst is used.

In the past the ClangFormatStyleOpions.rst has been generated from the Format.h via this script, we should break that.

The "In configuation" part is super important because it explains to user what to put into their .clang-format file.

We have to either have some form of markup that says LS_Cpp03 == c++03 in the documentation

[MyDeveloperDay]

clang/docs/tools/dump_format_style.py

175

*we shouldn't break that*

[mrexodia]

I don't really have a say in this, but I was trying to run the python script to generate the rst file and it didn't work. I tried fixing it, but it's definitely very hacky. Perhaps some kind of test can be introduced that runs this python script and checks if it succeeds, so that people are aware that their changes to Format.h break the documentation generation. I also need this to continue with https://reviews.llvm.org/D31635.

[MyDeveloperDay]

In D69433#1722463, @mrexodia wrote:

I don't really have a say in this, but I was trying to run the python script to generate the rst file and it didn't work. I tried fixing it, but it's definitely very hacky. Perhaps some kind of test can be introduced that runs this python script and checks if it succeeds, so that people are aware that their changes to Format.h break the documentation generation. I also need this to continue with https://reviews.llvm.org/D31635.

I agree, but first let us get them aligned again first, this revision goes some way to making that possible.

The remaining item from @mitchell-stellar's change I had an idea that we could put comments regarding the enums into a global dictionary of options, then if we see the enum used in the struct we could write the documentation out then, and then I think we'd be good, but I'd like to work on that change in isolation, and I don't want to do that with this one inflight still

Bottom line lets work out how we want to move forward after we are back in shape, at present we are out of sync and the script can't be used without wiping out previous documentation.

[mitchell-stellar]

Sounds like you know what you're doing.

LGTM

[sammccall]

Apologies for this mess, I didn't know about dump_format_style. Thanks for cleaning this up!

Generally the differences were intentional and I think the text in Format.h was mostly better :-( Happy to unify but a bunch of suggestions accordingly.

clang/docs/ClangFormatStyleOptions.rst

2336

Cpp03 is a deprecated alias for c++03.
The colons were just to give terse bullets, these are no longer bullets and I think they hurt readability.

2343

I'm not sure why this is grouped here. Did you intend this to be under LS_Latest?

2343

Cpp11 is a deprecated alias for Latest, for historical reasons.

clang/docs/tools/dump_format_style.py

175

The "In configuation" part is super important because it explains to user what to put into their .clang-format file.

Honestly, I'm not sure why the docs say "LS_Foo (in configuration: Foo)" rather than just "Foo" - why do users care what the enum is?

But this is an existing practice, and should be changed separately if at all.

clang/include/clang/Format/Format.h

1986–1987

I think "Parse and format as C++03" is better here. In the RST it's redundant, as the description for Standard precedes this block. But if we have to pick one, I'd go with "Parse and format as C++03".

2001

The new text (two lines) is better, please add it back.

2005–2009

nit: "Parse and Format" -> "Parse and format"

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

2343

yes correct I'll make that change with regard to its position.

clang/docs/tools/dump_format_style.py

175

I have a tendency to agree with you here.. who cares about the LK_ in the LK_Cpp value?

However I know as a clang-format developer I really care about seeing them from the perspective of being able to easily search in the code for things like BCIS_BeforeComma, otherwise, it's harder for me to work out which setting goes with which setting without going into Format.h and searching (but that's just me being lazy), but from a users perspective I wonder how many people put the enum name in the configuration by mistake..

Oh dear... it turns out this is a problem

https://github.com/search?l=YAML&q=LK_Cpp&type=Code

From time it time it appears people are using the enum name incorrectly.

@klimek maybe we should consider making this to make it a little clearer.

I feel we might be guiding people incorrectly.

[MyDeveloperDay]

Address a few grammatical review comments

After modifications

clang/include/clang/Format/Format.h

2001

you can't this breaks the script

clang/docs/ClangFormatStyleOptions.rst

**SpacesInParentheses** (``bool``)
 
   .. code-block:: c++
 
      true:                                  false:
      t f( Deleted & ) & = delete;   vs.     t f(Deleted &) & = delete;
 
 **SpacesInSquareBrackets** (``bool``)
   If ``true``, spaces will be inserted after ``[`` and before ``]``.
-  Lambdas without arguments or unspecified size array declarations will not be
-  affected.
+  Lambdas without arguments or unspecified size array declarations will not
+  be affected.
 
   .. code-block:: c++
 
      true:                                  false:
      int a[ 5 ];                    vs.     int a[5];
      std::unique_ptr<int[]> foo() {} // Won't be affected
 
 **Standard** (``LanguageStandard``)
   Parse and format C++ constructs compatible with this standard.
 
   .. code-block:: c++
 
      c++03:                                 latest:
      vector<set<int> > x;           vs.     vector<set<int>> x;
 
   Possible values:
 
   * ``LS_Cpp03`` (in configuration: ``c++03``)
-    Use C++03-compatible syntax.
+    Parse and format as C++03.
+    ``Cpp03`` is a deprecated alias for ``c++03``
 
   * ``LS_Cpp11`` (in configuration: ``c++11``)
-    Use C++11-compatible syntax.
+    Parse and format as C++11.
 
   * ``LS_Cpp14`` (in configuration: ``c++14``)
-    Use C++14-compatible syntax.
+    Parse and format as C++14.
 
   * ``LS_Cpp17`` (in configuration: ``c++17``)
-    Use C++17-compatible syntax.
+    Parse and format as C++17.
 
   * ``LS_Cpp20`` (in configuration: ``c++20``)
-    Use C++20-compatible syntax.
+    Parse and format as C++20.
 
   * ``LS_Latest`` (in configuration: ``Latest``)
     Parse and format using the latest supported language version.
+    ``Cpp11`` is a deprecated alias for ``Latest``
 
   * ``LS_Auto`` (in configuration: ``Auto``)
     Automatic detection based on the input.
 
-  * ``Cpp03``: deprecated alias for ``c++03``
 
-  * ``Cpp11``: deprecated alias for ``Latest``
 
 **StatementMacros** (``std::vector<std::string>``)
   A vector of macros that should be interpreted as complete
   statements.
 
   Typical macros are expressions, and require a semi-colon to be
   added; sometimes this is not the case, and this allows to make
   clang-format aware of such cases.

clang/docs/tools/dump_format_style.py

def __init__(self, name, comment):
     self.name = name
     self.comment = comment.strip()
     self.values = []
 
   def __str__(self):
     return '\n'.join(map(str, self.values))
 
 class EnumValue(object):
-  def __init__(self, name, comment):
+  def __init__(self, name, comment, config):
     self.name = name
     self.comment = comment
+    self.config = config
 
   def __str__(self):
     return '* ``%s`` (in configuration: ``%s``)\n%s' % (
         self.name,
-        re.sub('.*_', '', self.name),
+        re.sub('.*_', '', self.config),
         doxygen2rst(indent(self.comment, 2)))
 
 def clean_comment_line(line):
   match = re.match(r'^/// \\code(\{.(\w+)\})?$', line)
   if match:
     lang = match.groups()[1]
     if not lang:
       lang = 'c++'
elif state == State.InEnum:
         enums[enum.name] = enum
       else:
         raise Exception('Invalid format, expected enum field comment or };')
     elif state == State.InEnumMemberComment:
       if line.startswith('///'):
         comment += clean_comment_line(line)
       else:
         state = State.InEnum
-        enum.values.append(EnumValue(line.replace(',', ''), comment))
+        val = line.replace(',', '')
+        pos = val.find(" // ")
+        if (pos != -1):
+            config = val[pos+4:]
+            val = val[:pos]
+        else:
+            config = val;
+        enum.values.append(EnumValue(val, comment,config))
   if state != State.Finished:
     raise Exception('Not finished by the end of file')
 
   for option in options:
     if not option.type in ['bool', 'unsigned', 'int', 'std::string',
                            'std::vector<std::string>',
                            'std::vector<IncludeCategory>',
                            'std::vector<RawStringFormat>']:

clang/include/clang/Format/Format.h

enum BraceBreakingStyle {
     ///   {
     ///     A,
     ///     B
     ///   };
     /// \endcode
     BS_Allman,
     /// Like ``Allman`` but always indent braces and line up code with braces.
     /// \code
-    ///    try
+    ///   try
     ///     {
     ///     foo();
     ///     }
     ///   catch ()
     ///     {
     ///     }
     ///   void foo() { bar(); }
     ///   class foo
struct BraceWrappingFlags {
     ///   true:
     ///   class foo {};
     ///
     ///   false:
     ///   class foo
     ///   {};
     /// \endcode
     bool AfterClass;
+
     /// Wrap control statements (``if``/``for``/``while``/``switch``/..).
     BraceWrappingAfterControlStatementStyle AfterControlStatement;
     /// Wrap enum definitions.
     /// \code
     ///   true:
     ///   enum X : int
     ///   {
     ///     B

   /// If ``true``, spaces will be inserted after ``(`` and before ``)``.
   /// \code
   ///    true:                                  false:
   ///    t f( Deleted & ) & = delete;   vs.     t f(Deleted &) & = delete;
   /// \endcode
   bool SpacesInParentheses;
 
   /// If ``true``, spaces will be inserted after ``[`` and before ``]``.
-  /// Lambdas or unspecified size array declarations will not be affected.
+  /// Lambdas without arguments or unspecified size array declarations will not
+  /// be affected.
   /// \code
   ///    true:                                  false:
   ///    int a[ 5 ];                    vs.     int a[5];
   ///    std::unique_ptr<int[]> foo() {} // Won't be affected
   /// \endcode
   bool SpacesInSquareBrackets;
 
   /// Supported language standards for parsing and formatting C++ constructs.
   /// \code
   ///    Latest:                                vector<set<int>>
   ///    c++03                          vs.     vector<set<int> >
   /// \endcode
   ///
   /// The correct way to spell a specific language version is e.g. ``c++11``.
   /// The historical aliases ``Cpp03`` and ``Cpp11`` are deprecated.
   enum LanguageStandard {
-    /// c++03: Parse and format as C++03.
-    LS_Cpp03,
-    /// c++11: Parse and format as C++11.
-    LS_Cpp11,
-    /// c++14: Parse and format as C++14.
-    LS_Cpp14,
-    /// c++17: Parse and format as C++17.
-    LS_Cpp17,
-    /// c++20: Parse and format as C++20.
-    LS_Cpp20,
-    /// Latest: Parse and format using the latest supported language version.
-    /// 'Cpp11' is an alias for LS_Latest for historical reasons.
+    /// Parse and format as C++03.
+    /// ``Cpp03`` is a deprecated alias for ``c++03``
+    LS_Cpp03, // c++03
+    /// Parse and format as C++11.
+    LS_Cpp11, // c++11
+    /// Parse and format as C++14.
+    LS_Cpp14, // c++14
+    /// Parse and format as C++17.
+    LS_Cpp17, // c++17
+    /// Parse and format as C++20.
+    LS_Cpp20, // c++20
+    /// Parse and format using the latest supported language version.
+    /// ``Cpp11`` is a deprecated alias for ``Latest``
     LS_Latest,
-    /// Auto: Automatic detection based on the input.
-    /// Parse using the latest language version. Format based on detected input.
+    /// Automatic detection based on the input.
     LS_Auto,
   };
 
-  /// Format compatible with this standard, e.g. use ``A<A<int> >``
-  /// instead of ``A<A<int>>`` for ``LS_Cpp03``.
+  /// Parse and format C++ constructs compatible with this standard.
+  /// \code
+  ///    c++03:                                 latest:
+  ///    vector<set<int> > x;           vs.     vector<set<int>> x;
+  /// \endcode
   LanguageStandard Standard;
 
   /// The number of columns used for tab stops.
   unsigned TabWidth;
 
   /// Different ways to use tab in formatting.
   enum UseTabStyle {
     /// Never use tab.