[clang-format] NFC Document the other space before colon option

[sstwcw]

Summary

There are two options that do much the same thing, but for different
languages. With the addition to the doc, the user is less likely to
configure the wrong option and get frustrated that it doesn't work.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please try to ensure all code changes have a unit test (unless this is an NFC or refactoring, adding documentation etc..)

Add you unit tests in clang/unittests/Format and build ninja FormatTests we recommend using the verifyFormat(xxx) format of unit tests rather than EXPECT_EQ as this will ensure you change is tolerant to random whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can happen if you are trying to improve the annotation phase to ensure we are correctly identifying the type of a token, please add a token annotator test in TokenAnnotatorTest.cpp

[owenpan]

Should we extend SpacesInContainerLiterals so that it controls JSON colons too? If yes, then we don't need SpaceBeforeJsonColon. Otherwise, IMO we should leave the doc alone.

[MyDeveloperDay]

In D147422#4240021, @owenpan wrote:

Should we extend SpacesInContainerLiterals so that it controls JSON colons too? If yes, then we don't need SpaceBeforeJsonColon. Otherwise, IMO we should leave the doc alone.

My concern for SpacesInContainerLiterals is that it impacts arrays contents too '[ 1, 2, 3 ]'.

[MyDeveloperDay]

In D147422#4239701, @Herald wrote:

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please try to ensure all code changes have a unit test (unless this is an NFC or refactoring, adding documentation etc..)

Add you unit tests in clang/unittests/Format and build ninja FormatTests we recommend using the verifyFormat(xxx) format of unit tests rather than EXPECT_EQ as this will ensure you change is tolerant to random whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can happen if you are trying to improve the annotation phase to ensure we are correctly identifying the type of a token, please add a token annotator test in TokenAnnotatorTest.cpp

Thanks for testing this automated comment ;-) , I've fixed some of the grammatical issue and made it so it won't fire if NFC is in the title.

[sstwcw]

In D147422#4240024, @MyDeveloperDay wrote:

In D147422#4240021, @owenpan wrote:

Should we extend SpacesInContainerLiterals so that it controls JSON colons too? If yes, then we don't need SpaceBeforeJsonColon. Otherwise, IMO we should leave the doc alone.

My concern for SpacesInContainerLiterals is that it impacts arrays contents too '[ 1, 2, 3 ]'.

It looks like a line break gets inserted in arrays. Does that mean the option doesn't affect arrays?

[owenpan]

In D147422#4240024, @MyDeveloperDay wrote:

In D147422#4240021, @owenpan wrote:

Should we extend SpacesInContainerLiterals so that it controls JSON colons too? If yes, then we don't need SpaceBeforeJsonColon. Otherwise, IMO we should leave the doc alone.

My concern for SpacesInContainerLiterals is that it impacts arrays contents too '[ 1, 2, 3 ]'.

It already affects JSON arrays if BreakArrays is set to false:

$ cat foo.json
[1, 2, 3]
$ clang-format -style='{BreakArrays: false}' foo.json
[ 1, 2, 3 ]

[owenpan]

In D147422#4242115, @sstwcw wrote:

It looks like a line break gets inserted in arrays. Does that mean the option doesn't affect arrays?

It matters only if BreakArrays is set to false.

clang/docs/ClangFormatStyleOptions.rst

**SpaceBeforeInheritanceColon** (``Boolean``) :versionbadge:`clang-format 7` :ref:`¶ <SpaceBeforeInheritanceColon>`
   .. code-block:: c++
 
      true:                                  false:
      class Foo : Bar {}             vs.     class Foo: Bar {}
 
 .. _SpaceBeforeJsonColon:
 
 **SpaceBeforeJsonColon** (``Boolean``) :versionbadge:`clang-format 17` :ref:`¶ <SpaceBeforeJsonColon>`
-  If ``true``, a space will be add before a JSON colon.
+  If ``true``, a space will be added before a JSON colon. For other
+  languages, e.g. JavaScript, use ``SpacesInContainerLiterals`` instead.
 
   .. code-block:: c++
 
      true:                                  false:
      {                                      {
        "key" : "value"              vs.       "key": "value"
      }                                      }
 
.. code-block:: c++
 
      true:                                  false:
      if ( a )  { ... }              vs.     if (a) { ... }
      while ( i < 5 )  { ... }               while (i < 5) { ... }
 
 .. _SpacesInContainerLiterals:
 
 **SpacesInContainerLiterals** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`¶ <SpacesInContainerLiterals>`
-  If ``true``, spaces are inserted inside container literals (e.g.
-  ObjC and Javascript array and dict literals).
+  If ``true``, spaces are inserted inside container literals (e.g.  ObjC and
+  Javascript array and dict literals). For JSON, use
+  ``SpaceBeforeJsonColon`` instead.
 
   .. code-block:: js
 
      true:                                  false:
      var arr = [ 1, 2, 3 ];         vs.     var arr = [1, 2, 3];
      f({a : 1, b : 2, c : 3});              f({a: 1, b: 2, c: 3});
 
 .. _SpacesInLineCommentPrefix:

clang/include/clang/Format/Format.h

   /// If ``false``, spaces will be removed before inheritance colon.
   /// \code
   ///    true:                                  false:
   ///    class Foo : Bar {}             vs.     class Foo: Bar {}
   /// \endcode
   /// \version 7
   bool SpaceBeforeInheritanceColon;
 
-  /// If ``true``, a space will be add before a JSON colon.
+  /// If ``true``, a space will be added before a JSON colon. For other
+  /// languages, e.g. JavaScript, use ``SpacesInContainerLiterals`` instead.
   /// \code
   ///    true:                                  false:
   ///    {                                      {
   ///      "key" : "value"              vs.       "key": "value"
   ///    }                                      }
   /// \endcode
   /// \version 17
   bool SpaceBeforeJsonColon;

   /// \code
   ///    true:                                  false:
   ///    if ( a )  { ... }              vs.     if (a) { ... }
   ///    while ( i < 5 )  { ... }               while (i < 5) { ... }
   /// \endcode
   /// \version 10
   bool SpacesInConditionalStatement;
 
-  /// If ``true``, spaces are inserted inside container literals (e.g.
-  /// ObjC and Javascript array and dict literals).
+  /// If ``true``, spaces are inserted inside container literals (e.g.  ObjC and
+  /// Javascript array and dict literals). For JSON, use
+  /// ``SpaceBeforeJsonColon`` instead.
   /// \code{.js}
   ///    true:                                  false:
   ///    var arr = [ 1, 2, 3 ];         vs.     var arr = [1, 2, 3];
   ///    f({a : 1, b : 2, c : 3});              f({a: 1, b: 2, c: 3});
   /// \endcode
   /// \version 3.7
   bool SpacesInContainerLiterals;