MyDeveloperDay requested changes to this revision.
MyDeveloperDay added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang/include/clang/Format/Format.h:1950
+ /// similar) conditions.
+ bool SpacesAroundConditions;
+
----------------
its position in the file and the documentation is not quite alphabetic, given
you are close i'd make it so
================
Comment at: clang/include/clang/Format/Format.h:2088
SpacesInSquareBrackets == R.SpacesInSquareBrackets &&
+ SpacesAroundConditions == R.SpacesAroundConditions &&
Standard == R.Standard && TabWidth == R.TabWidth &&
----------------
move upto just before SpacesBeforeTrailingComments
================
Comment at: clang/lib/Format/Format.cpp:520
IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
+ IO.mapOptional("SpacesAroundConditions", Style.SpacesAroundConditions);
IO.mapOptional("Standard", Style.Standard);
----------------
Same here
================
Comment at: clang/lib/Format/Format.cpp:779
LLVMStyle.SpacesInAngles = false;
+ LLVMStyle.SpacesAroundConditions = false;
----------------
Sorry move to line 771-772
================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2509
+ if (Style.SpacesAroundConditions) {
+ const auto is_cond_kw = [&](const FormatToken *t) {
+ return t->isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
----------------
this isn't really the normal naming convention for variables, I think it would
be `IsCondKw` I think its clearer without using abbreviations,
why not just make this a function? does the Lambda really give us anything here
in such a big function other than clutter?
`static bool isConditionKeyword(....)`
================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2919
+ if (Right.is(tok::coloncolon) &&
+ !Left.isOneOf(tok::l_brace, tok::comment, tok::l_paren))
return (Left.is(TT_TemplateOpener) &&
----------------
I'm not sure I understand this change so you added a `tok::l_paren` here? but
its not just for your style, so something else must have changed. Did you run
all FormatTests?
one of the tests you add need to test why you added this here
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68346/new/
https://reviews.llvm.org/D68346
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
