[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

kadircet added a comment. The changes are not directly related to clang-tidy, so could you get rid of that clang-tidy some-checker-specific tests and put some unittests into clang/unittests/Format/CleanupTest.cpp ? Comment at: clang/include/clang/Basic/CharInfo.h:196 +/// R

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

kadircet added a comment. also could you rename the revision so that it reflects the fact that, this is a change to clang-format and has nothing to do with clang-tidy ? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68682/new/ https://reviews.llvm.o

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc marked 4 inline comments as done. poelmanc added inline comments. Comment at: clang/lib/AST/CommentLexer.cpp:20 + +// Consider moving this useful function to a more general utility location. +const char *skipNewline(const char *BufferPtr, const char *BufferEnd) { ---

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc updated this revision to Diff 228795. poelmanc edited the summary of this revision. poelmanc added a comment. Move isWhitespace and skipNewlines to clang/Basic/CharInfo.h (renaming to isWhitespaceStringRef and skipNewlinesChars to resolve name clashes) and add double-quotes around "/n"

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

gribozavr2 added inline comments. Comment at: clang/include/clang/AST/CommentLexer.h:25 + +/// Requires that BufferPtr point to a newline character (/n or /r). +/// Returns a pointer past the end of any platform newline, i.e. past "\n" and "\r" (everywhere) ===

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc updated this revision to Diff 228558. poelmanc added a comment. I just rebased this patch on the latest master. I believe I've addressed all the comments raised so far. Should I add mention of this change to the release notes? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST A

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc updated this revision to Diff 225523. poelmanc edited the summary of this revision. poelmanc added a comment. In D68682#1707764 , @alexfh wrote: > cleanupAroundReplacements is not used by clang-format itself. I believe, it's > a part of clang-for

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

alexfh added a comment. In D68682#1706581 , @poelmanc wrote: > Thanks, from the name that sounds like the perfect place to do it. If > `cleanupAroundReplacements` also is used by clang-format, would we want to > make the functionality optional, e.g. via

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc added a comment. In D68682#1705866 , @gribozavr wrote: > > I guess here's the high-level question: should all removals that remove all > > non-blank text from a line also delete the line? > > I see your point, but as https://llvm.org/PR43583 show

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc added a comment. In D68682#1705865 , @Eugene.Zelenko wrote: > In D68682#1705505 , @jonathanmeier > wrote: > > > In D68682#1702025 , @poelmanc > > wrote: > > > > >

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

Eugene.Zelenko added a comment. In D68682#1705505 , @jonathanmeier wrote: > In D68682#1702025 , @poelmanc wrote: > > > In D68682#1700908 , > > @Eugene.Zelenko wrote: > > >

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

gribozavr added a comment. > I guess here's the high-level question: should all removals that remove all > non-blank text from a line also delete the line? I see your point, but as https://llvm.org/PR43583 shows, we have a much larger problem: textual replacements don't compose. So, whatever we

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

jonathanmeier added a comment. In D68682#1702025 , @poelmanc wrote: > In D68682#1700908 , @Eugene.Zelenko > wrote: > > > You may be interested to also look on PR43583 related to > > readability-redundant-member-in

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc marked an inline comment as done. poelmanc added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:27 #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/CharInfo.h" // for isWhiteSpace(char), isVerticalWhitespace(char) #include

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

MyDeveloperDay added a comment. @poelmanc thank you for the very detailed explanation to what was probably my very naive point, I have to say with more information I am inclined to agree with you that perhaps you need to see all the replacements as a whole to make reasonable assumptions. But I

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc added a comment. In D68682#1700908 , @Eugene.Zelenko wrote: > You may be interested to also look on PR43583 related to > readability-redundant-member-init. Thanks Eugene, I'm having trouble finding that. https://reviews.llvm.org/D43583 seems

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc updated this revision to Diff 224089. poelmanc retitled this revision from "Clang-tidy fixes should avoid creating new blank lines" to "Clang-tidy fix removals removing all non-blank text from a line should remove the line". poelmanc edited the summary of this revision. poelmanc added a