vlovich added inline comments.
================ Comment at: clang/docs/ReleaseNotes.rst:252 +- Option ``IfMacros`` has been added. This lets you define macros that get + formatted like conditionals much like ``ForEachMacros`` get stiled like + foreach loops. ---------------- MyDeveloperDay wrote: > stiled? did you mean styled? Whoops. Yes I did. ================ Comment at: clang/lib/Format/FormatTokenLexer.cpp:1019 FormatTok->setType(it->second); + if (it->second == TT_IfMacro) { + FormatTok->Tok.setKind(tok::kw_if); ---------------- MyDeveloperDay wrote: > Is there any chance you could leave a comment here as to why this is needed, > I can't work it out? Will do my best. TLDR: The token is otherwise "unknown" & the conditional formatting logic depends on the lexer token value. This avoids having to rewrite all the existing code & seemed to make sense since we want this token treated like an `if`. I don't of course know if this is The Best Way (tm). It's just the minimal change I figured out would get the formatting to work properly but happy to apply feedback from someone more intimately familiar with the internals of the formatter as I'm a complete novice here. ================ Comment at: clang/lib/Format/TokenAnnotator.cpp:3025 return false; + if (Left.is(TT_IfMacro)) { + return false; ---------------- MyDeveloperDay wrote: > I'll let you decide if you think we need another SBPO_XXX style? I thought about it but I wasn't was really sure how to add it in a way that would make sense. Do you think people would want to apply consistent SBPO styling for IF & FOREACH macros or want fine-grained control? If the former, then I can just check the foreach macro & maybe rename it to `SBPO_ControlStatementsExceptMacros` (maintaining the old name for back compat). If the latter, then it would seem like we need a separate boolean that controls whether SBPO_ControlStatements would apply? My gut is probably the "maintain consistency" option is fine for now so I've gone ahead & applied that change in the latest diff. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102730/new/ https://reviews.llvm.org/D102730 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits