mprobst 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:
> > 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.
> I cannot decide whether I must break in `mustBreak` - I need to just force
> wrapping after `${` if I need to wrap somewhere below, but if the code fits
> on the line, I don't want to wrap. So when I'm visiting `${` I cannot decide
> that, and when I'm at the object literal expression, the decision for `${`
> has already been made - right? Or am I missing something?
I've dropped the function wrapping fix and will land like this, apparently
we'll need to think about this a bit more.
https://reviews.llvm.org/D37142
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
