[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 326957. poelmanc added a comment. Removed a function in modernize/LoopConvertCheck.cpp that's no longer needed due to this patch. Fixed clang-format and clang-tidy issues identified by HarborMaster in prior submission. Hopefully it's ready to go, but as

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc added a comment. In D68682#2547906 , @kadircet wrote: > It looks like premerge tests skipped your last diff with id 321451 > (https://reviews.llvm.org/harbormaster/?after=87950). You can re-trigger by > uploading a new diff, in the meantime i

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 322265. poelmanc added a comment. Call `llvm::Annoation` constructor rather than `operator=` to fix linux build issue, fix some issues identified by clang-format and clang-tidy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 322108. poelmanc added a comment. Comment change, "beginning" to "start" for consistency, being sure to set Repository on "diff" page (not just on edit page) to see if https://github.com/google/llvm-premerge-checks/issues/263 was the problem. Repository:

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 322033. poelmanc added a comment. Update one one comment, hopefully trigger HarborMaster to rerun. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68682/new/ https://reviews.llvm.org/D68682 Files:

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

kadircet added a comment. It looks like premerge tests skipped your last diff with id 321451 (https://reviews.llvm.org/harbormaster/?after=87950). You can re-trigger by uploading a new diff, in the meantime i would also file a bug to https://github.com/google/llvm-premerge-checks/issues.

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc added a comment. On 2 Feb Harbormaster found a bug from my switching some char* code to use StringRef. I uploaded a new patch on 4 Feb, but Harbormaster does not appear to have rerun. What triggers Harbormaster - do I need to take some action? I

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 321451. poelmanc added a comment. Herald added a subscriber: nullptr.cpp. Change loop end condition in `findLineEnd` and add several `assert` statements. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68682/new/ https://reviews.llvm.org/D68682

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc marked 14 inline comments as done. poelmanc added inline comments. Comment at: clang/lib/Format/Format.cpp:2381 +// if so adds a Replacement to NewReplacements that removes the entire line. +llvm::Error MaybeRemoveBlankLine(tooling::Replacements , +

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 320678. poelmanc changed the repository for this revision from rCTE Clang Tools Extra to rG LLVM Github Monorepo. poelmanc added a comment. Herald added subscribers: dexonsmith, mgorny. Glad to be back after a year away from clang-tidy, and sorry to have

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang/lib/Format/Format.cpp:2381 +// if so adds a Replacement to NewReplacements that removes the entire line. +llvm::Error MaybeRemoveBlankLine(tooling::Replacements , +

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc marked 4 inline comments as done. poelmanc added a comment. In D68682#1754798 , @kadircet wrote: > `ToolingTests/ApplyAtomicChangesTest.FormatsCorrectLineWhenHeaderIsRemoved` > also seems to be failing, you can run it with `ninja ToolingTests &&

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc marked an inline comment as done. poelmanc added inline comments. Comment at: clang/include/clang/Basic/CharInfo.h:94 +LLVM_READONLY inline bool isWhitespace(llvm::StringRef S) { + for (StringRef::const_iterator I = S.begin(), E = S.end(); I != E; ++I) { +if

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc marked 13 inline comments as done. poelmanc added inline comments. Comment at: clang/lib/Format/Format.cpp:2385 + const char *FileText = Code.data(); + StringRef FilePath; // Must be the same for every Replacement + for (const auto : Replaces) {

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 235090. poelmanc marked 2 inline comments as done. poelmanc added a comment. Fix algorithm to fix failing `ToolingTests/ApplyAtomicChangesTest.FormatsCorrectLineWhenHeaderIsRemoved`. That test revealed a very specific but real failure case: if existing

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 234976. poelmanc added a comment. Address most of the feedback, I'll comment individually. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68682/new/ https://reviews.llvm.org/D68682 Files:

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

alexfh added inline comments. Comment at: clang/include/clang/Basic/CharInfo.h:94 +LLVM_READONLY inline bool isWhitespace(llvm::StringRef S) { + for (StringRef::const_iterator I = S.begin(), E = S.end(); I != E; ++I) { +if (!isWhitespace(*I)) I'd suggest to

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

kadircet added a comment. `ToolingTests/ApplyAtomicChangesTest.FormatsCorrectLineWhenHeaderIsRemoved` also seems to be failing, you can run it with `ninja ToolingTests && ./tools/clang/unittests/Tooling/ToolingTests --gtest_filter="ApplyAtomicChangesTest.FormatsCorrectLineWhenHeaderIsRemoved"`

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc marked an inline comment as done. poelmanc added a comment. In D68682#1753357 , @kadircet wrote: > 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 ?

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc marked 5 inline comments as done. poelmanc added inline comments. Comment at: clang/lib/AST/CommentParser.cpp:19 -static inline bool isWhitespace(llvm::StringRef S) { +// Consider moving this useful function to a more general utility location. +bool

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 230375. poelmanc marked 3 inline comments as done. poelmanc retitled this revision from "Clang-tidy fix removals removing all non-blank text from a line should remove the line" to "format::cleanupAroundReplacements removes whole line when Removals leave