djasper added inline comments.

================
Comment at: lib/Format/ContinuationIndenter.cpp:1139
+
+  // On lines containing template strings, propagate NoLineBreak even for dict
+  // and array literals. This is to force wrapping an initial function call if
----------------
mprobst wrote:
> djasper wrote:
> > This is not the right way to implement this:
> > 
> > - This is a static computation that we could do ahead of time. Doing it 
> > inside the combinatorial exploration of solutions is a waste.
> > - You are doing this always, even in code that doesn't have template 
> > strings or isn't even JavaScript.
> > - This can lead to unexpected behavior if the template string is in a 
> > completely unrelated part of the statement. E.g.
> > 
> > 
> >   someFunction(`test`, { ... });
> > 
> > will be formatted differently from
> > 
> >   someFunction('test', { ... });
> Ack. Do you have suggestions?
> 
> I could introduce a `bool  ParenState::NoLineBreakMustPropagate;` that 
> controls this behaviour.
That, too, would create a significant memory and runtime overhead to everyone 
just to fix this rather minor formatting detail.
If we were to set MatchingParen correctly to match up "${" to "}", we could 
measure the respective length and at a very early state (mustBreak()) decide 
that we need a line break between "${" and the corresponding "}". Now setting 
that correctly might be hard. Maybe it is ok, to linearly scan in that case as 
we would only do that if we actually find a template string ending in "${" and 
the distance to the "}" is usually short.


https://reviews.llvm.org/D37142



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to