owenpan added inline comments.
================
Comment at: clang/lib/Format/Format.cpp:3893
+bool isClangFormatOn(StringRef Comment) {
+ if (Comment == "/* clang-format on */")
----------------
alexolog wrote:
> alexolog wrote:
> > Here's my attempt at something flexible:
> > Disclaimer: this is my first time looking at the LLVM codebase, so don't
> > expect production quality.
> >
> > ```
> > ////////////////////////////////////////////////////////////////////////////////////////////////////
> >
> > // Implementation detail, hide in a namespace and/or take out of the header
> > bool isClangFormatMarked(StringRef Comment, StringRef Mark) {
> > // Quick heuristics: minimum length and starts with a slash (comment)
> > // Shortest tag: "//clang-format on", 17 characters
> > static constexpr StringLiteral clangFormatStr("clang-format ");
> > if (Comment.size() < clangFormatStr.size() + 4 || Comment[0] != '/')
> > return false;
> >
> > // check if it's a comment starting with "//" or "/*"
> > bool CloseNeeded = false;
> > if (Comment[1] == '*')
> > CloseNeeded = true;
> > else if (Comment[1] != '/')
> > return false;
> >
> > // Remove the comment start and all following whitespace
> > Comment = Comment.substr(2).ltrim();
> >
> > // Check for the actual command, a piece at a time
> > if (!Comment.consume_front(clangFormatStr) ||
> > !Comment.consume_front(Mark))
> > return false;
> >
> > // Are we there yet?
> > if (!CloseNeeded && Comment.empty() ||
> > CloseNeeded && Comment.starts_with("*/"))
> > return true;
> >
> > // For a trailer, restrict the next character
> > // (currently spaces and tabs, but can include a colon, etc.)
> > static constexpr StringLiteral Separator(" \t");
> > if (!Separator.contains(Comment[0]))
> > return false;
> >
> > // Verify comment is properly terminated
> > if (!CloseNeeded || Comment.contains("*/"))
> > return true;
> >
> > return false; // Everything else
> > }
> >
> > ////////////////////////////////////////////////////////////////////////////////////////////////////
> >
> > bool isClangFormatOn(StringRef Comment) {
> > return isClangFormatMarked(Comment, "on");
> > }
> >
> > bool isClangFormatOff(StringRef Comment) {
> > return isClangFormatMarked(Comment, "off");
> > }
> >
> > ////////////////////////////////////////////////////////////////////////////////////////////////////
> >
> > EXPECT_TRUE(isClangFormatOn("//clang-format on"));
> > EXPECT_TRUE(isClangFormatOn("// clang-format on"));
> > EXPECT_TRUE(isClangFormatOn("//clang-format on "));
> > EXPECT_TRUE(isClangFormatOn("//clang-format on and off"));
> > EXPECT_TRUE(isClangFormatOn("/*clang-format on*/"));
> > EXPECT_TRUE(isClangFormatOn("/* clang-format on*/"));
> > EXPECT_TRUE(isClangFormatOn("/*clang-format on */"));
> > EXPECT_TRUE(isClangFormatOn("/*clang-format on*/int i{0};"));
> >
> > EXPECT_FALSE(isClangFormatOn("//clang-format on"));
> > EXPECT_FALSE(isClangFormatOn("//clang-format o"));
> > EXPECT_FALSE(isClangFormatOn("// clang-format o"));
> > EXPECT_FALSE(isClangFormatOn("//clang-format ontario"));
> > EXPECT_FALSE(isClangFormatOn("//clang-format off"));
> > EXPECT_FALSE(isClangFormatOn("/*clang-format onwards*/"));
> >
> > ////////////////////////////////////////////////////////////////////////////////////////////////////
> > ```
> Sorry about the "done". My misunderstanding
> Here's my attempt at something flexible:
> Disclaimer: this is my first time looking at the LLVM codebase, so don't
> expect production quality.
Thanks! If we didn't have to worry about regressions, we might want to do
something like what you suggested above.
================
Comment at: clang/lib/Format/Format.cpp:3901
+ return Comment.startswith(Prefix) &&
+ (Comment.size() == Size || isblank(Comment[Size]));
+}
----------------
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142804/new/
https://reviews.llvm.org/D142804
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
