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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
