[PATCH] D80950: [clang-format] [PR44542,38872] String << String always get a forced newline.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D80950#2072926 <https://reviews.llvm.org/D80950#2072926>, @MyDeveloperDay wrote:

> The same function with smaller variables isn't quite so readable
>
>   LOG_IF(DFATAL, a < b) << "Equality condition can never be satisfied:"
>                         << " foo=" << a << " bar=" << b;
>


FWIW, this looks fine to me, it's the split between = and variable I was objecting to motsly.
We could still get unlucky depending on line lengths:

  LOG << "blah"
    << "foo=" << foo << " bar="
    << b;

In which case I'd probably split the space away from bar, writing the second line as `"<< "foo=" << foo << " "`...

> and there is inconsistency between
> 
>   LOG_IF(DFATAL, a < b) << "Equality condition can never be satisfied:"
>                         << " foo=" << a << " is less than "
>                         << " bar=" << b;
> 
> 
> 
> 
>   std::string reason = " is less than ";
>   LOG_IF(DFATAL, a < b) << "Equality condition can never be satisfied:"
>                         << " foo=" << a << reason << " bar=" << b;

Maybe I'm used to it, but that seems like a natural difference rather than an annoying inconsistency to me.
(The latter is clearly less readable with or without formatting, precisely *because* it's pasting together variables that we have to understand the semantics of to get any understanding).

>   std::string reason = " is less than ";
>   LOG_IF(DFATAL, a < b) << "=" << " foo=" << a << reason << " bar=" << b;
> 
> 
> because that actually looks a little better IMHO than
> 
>   std::string reason = " is less than ";
>   LOG_IF(DFATAL, a < b) << "="
>                         << " foo=" << a << reason << " bar=" << b;

This is clearly a question of taste :-)

An option seems attractive here. We haven't met the bar of multiple competing published style guides, but here the historical behaviour is idiosyncratic but long-standing and does some things people want...

I'd ask that we call the existing behavior something flexible like `Heuristic` or so... e.g. today we break after `endl`, and we should probably extend that to strings ending in `"\n"` since endl is often silly.
(If I was actually in a position to hack on clang-format more, I'd want to look at having a lower penalty when strings end in `=` etc, to encourage semantic breaks)

> But whatever we do I think we should leave the current one as the default.

I think we'd like to keep it for at least Google style. Feel less strongly about the others, but if an option is added we might as well separate the implementation from the flag-flip.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80950