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

Wed Oct 9 13:10:20 PDT 2019

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 appreciate you making the change to accommodate my concern.

As I'm not over here in clang-tidy land very often I will leave you to the others (and they give very detailed reviews), so I'll wish you good luck and if it helps make clang-tidy and clang-format work better together then you definitely have my vote!

================
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 "clang/Config/config.h"
----------------
Nit: I'm guessing you'd drop these comments later

================
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:143
+                     ReplacementText.empty();
+    LineCheckedThroughPos = R.getOffset() + R.getLength();
+  }
----------------
so If I understand correctly, if the ReplcementText is empty we are assuming we've removed it, as opposed to if someone was adding a blank line the ReplacementText would be all whitespace as we might remove it by mistake.. sound reasonable

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682