[PATCH] D142804: [clang-format] Support clang-format on/off line comments as prefixes

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:3901
+  return Comment.startswith(Prefix) &&
+         (Comment.size() == Size || isblank(Comment[Size]));
+}
----------------
HazardyKnusperkeks wrote:
> owenpan wrote:
> > HazardyKnusperkeks wrote:
> > > rymiel wrote:
> > > > Should the space be required? What about `// clang-format off: reasoning` or similar?
> > > > 
> > > > Also, should this be documented?
> > > > Should the space be required? What about `// clang-format off: reasoning` or similar?
> > > > 
> > > > Also, should this be documented?
> > > 
> > > +1
> > > Should the space be required? What about `// clang-format off: reasoning` or similar?
> > 
> > On second thought, we should make it more restrictive to avoid regressions. How about //requiring// a colon, i.e. `// clang-format off:` (but not `// clang-format off :`)?
> >  
> > > Also, should this be documented?
> > 
> > Yep.
> > 
> > > Should the space be required? What about `// clang-format off: reasoning` or similar?
> > 
> > On second thought, we should make it more restrictive to avoid regressions. How about //requiring// a colon, i.e. `// clang-format off:` (but not `// clang-format off :`)?
> >  
> 
> That's fine by me. But why not also `/**/`?
> But why not also `/**/`?

If it weren't for the fact that the code which checks for clang-format on/off exists in several places, I wouldn't want this feature added. IMO there's no need to allow `/* clang-format off: */` if we got `// clang-format:` and being more restrictive results in a lower risk of regression.


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

https://reviews.llvm.org/D142804