[PATCH] D80950: [clang-format] [PR44542,38872] String << String always get a forced newline.
MyDeveloperDay via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 3 02:43:10 PDT 2020
MyDeveloperDay added a comment.
In D80950#2069733 <https://reviews.llvm.org/D80950#2069733>, @curdeius wrote:
> The change seems to me technically sound, but I'm not sure of the scope of its effects. There might be users that rely on this behavior. On the other hand, adding an option to keep the old behavior doesn't seem appropriate, and personally I consider the old behavior as a bug.
I also have concerns about my own fix. This will break existing code similar to that found in UnwrappedLineParser.cpp that I included here, whilst I also think this is a bug, I fear users will complain that we broke compatibility. The fact it lacked tests was also a shock given how vigorously that is defended when people add new functionality.
One thing that perhaps we could consider is keeping the old odd behavior but behind a new configuration option, declaring it an outright bug and removing it might just be a more subjective than an objective opinion (if I think about the longevity of this 7+year and remove my myopic view that its a bug I can see that some code could be made less readable by this change)
What do people think about if we added
enum ChevronBreakingStyle
{
Never, // at line limit only
LegacyBetweenStrings, // (current)
AfterEndOrNewLine, // we could break on endl/std::endl/ends/std::ends/"\n" or any string literal ending with \n
Always // like between strings but between all << types and << "string"
}
Even if the default was LegacyBetweenStrings, I think those that are concerned about how it currently works would likely be happier to make the change to get the behavior they wanted, that way we wouldn't even break anywone.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80950/new/
https://reviews.llvm.org/D80950
More information about the cfe-commits
mailing list