poelmanc added a comment. In D68682#1705866 <https://reviews.llvm.org/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 shows, we have a much > larger problem: textual replacements don't compose. So, whatever we do, it > will only be hacky workarounds for specific high-value cases. > > About how exactly to do it. It would be preferred if checkers that already > know that they are deleting a full line, just deleted the newline characters. > However, for other cases, I feel like the place where this patch implements > newline deletion is not ideal. It implements newline deletion while applying > all fixes. Applying fixes is not the only client of fixit information. > Probably it is one of the least-important ones, to be honest. I don't know > who will run clang-tidy and just trust it to apply whatever fixes it wants. > More likely, the user will want to see a preview of findings and fixes, and > fixit application will be done by some other tool. Therefore, implementing > newline deletion in the code that applies all fixes is not going to be > helpful for most workflows. > > Therefore, I think the most appropriate place to do this cleanup is > `cleanupAroundReplacements`. What do you think? 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 a new bool parameter to `cleanupAroundReplacements`, a new option in FormatStyle, etc.? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68682/new/ https://reviews.llvm.org/D68682 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
