[PATCH] D71659: [clang-format] Added new option to allow setting spaces before and after the operator

[Owen Pan via Phabricator via cfe-commits]

owenpan commandeered this revision.
owenpan edited reviewers, added: Luis; removed: owenpan.
Herald added a project: All.
Herald added reviewers: rymiel, HazardyKnusperkeks.
Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

Your review contains a change to clang/include/clang/Format/Format.h but does 
not contain an update to ClangFormatStyleOptions.rst

ClangFormatStyleOptions.rst is generated via 
clang/docs/tools/dump_format_style.py,  please run this to regenerate the .rst

You can validate that the rst is valid by running.

  ./docs/tools/dump_format_style.py
  mkdir -p html
  /usr/bin/sphinx-build -n ./docs ./html


Herald added a comment.

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 your unit tests in `clang/unittests/Format` and you can build with `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`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71659/new/

https://reviews.llvm.org/D71659

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71659: [clang-format] Added new option to allow setting spaces before and after the operator

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3359
+  // nuanced as aggressive line breaks are placed when the lambda is not the
+  // last arg.
   if ((Style.Language == FormatStyle::LK_Cpp ||

these are all unrelated changes please remove them from patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71659/new/

https://reviews.llvm.org/D71659



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71659: [clang-format] Added new option to allow setting spaces before and after the operator

[Pablo Martin-Gomez via Phabricator via cfe-commits]

Bouska added a comment.

Is there a reason why this option adds space before and after the tok::arrow 
token but not the tok::arrowstar token? That seems inconsistent to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71659/new/

https://reviews.llvm.org/D71659



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71659: [clang-format] Added new option to allow setting spaces before and after the operator

[Luis Pinto via Phabricator via cfe-commits]

Luis added a comment.

In D71659#1789694 , @mitchell-stellar 
wrote:

> This feature is missing unit tests. Also, what is the reason for all the 
> comment changes? I don't think the changed comment lines originally exceeded 
> 80 characters.


Yup, I will add the unit-test when I have time, regarding the comments, it's 
really weird because I'm using clang-format to format the file on save, I'll 
double check and try to revert those comment changes!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71659/new/

https://reviews.llvm.org/D71659



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71659: [clang-format] Added new option to allow setting spaces before and after the operator

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar requested changes to this revision.
mitchell-stellar added a comment.
This revision now requires changes to proceed.

This feature is missing unit tests. Also, what is the reason for all the 
comment changes? I don't think the changed comment lines originally exceeded 80 
characters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71659/new/

https://reviews.llvm.org/D71659



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits