HazardyKnusperkeks requested review of this revision.
HazardyKnusperkeks added inline comments.
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3756
+ * ``bool AfterRequiresClause`` If ``true``, put space between requires
keyword in a requires clause and
+ opening parentheses, if is are one.
+
----------------
Quuxplusone wrote:
> HazardyKnusperkeks wrote:
> > Quuxplusone wrote:
> > > HazardyKnusperkeks wrote:
> > > > curdeius wrote:
> > > > > You meant "if there is one", right?
> > > > Yeah
> > > IMO these options should be named `InRequiresClause` and
> > > `InRequiresExpression`: that's where the space is going. The space
> > > doesn't go //after// the requires-clause. The space doesn't go //after//
> > > the requires-expression.
> > > It occurs to me that the name of this option could be analogous to the
> > > name of the option that controls `[]() {}` versus `[] () {}`... except
> > > that it looks like there is no such option (and I'm happy about that). :)
> > > Also, the name of that option would probably just be `AfterCaptureList`,
> > > which doesn't help us in this case.
> > I get your point, but the space does not go anywhere in the
> > clause/expression, so `AfterRequiresForClauses`?
> (I'd avoid the word `For`, because of keyword `for`.)
> A //requires-expression// is the whole expression, `requires + param-list +
> body`: https://eel.is/c++draft/expr.prim.req#nt:requires-expression
> A //requires-clause// is the whole clause, `requires + logical-or-expr`:
> https://eel.is/c++draft/temp.pre#nt:requires-clause
> Does that resolve your concern about the word `In`?
My concern is: `In` may refer to anywhere in the clause, but we are talking
about one very specific point per clause. (Analogues for expression.)
The current version is very explicit, and I don't have a problem with such a
long name.
================
Comment at: clang/include/clang/Format/Format.h:3371
+ /// If ``true``, put space between requires keyword in a requires clause
and
+ /// opening parentheses, if there is one.
+ /// \code
----------------
Quuxplusone wrote:
> Here and line 3380, and possibly elsewhere: `s/parentheses/parenthesis/`
Non native here, but I think one could argue both were okay. I just copied what
the other options state.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113369/new/
https://reviews.llvm.org/D113369
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
