<div dir="ltr">Yes, you are right. It definitely seems to have a non-trivial cost to change this and I don't think it is worth the effort then.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Nov 21, 2013 at 10:47 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div class="im">On Thu, Nov 21, 2013 at 6:54 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">In ContinuationIndenter::mustBreak().. clang-format does not use that code at all if everything fits on one line..</p>
</blockquote></div><div>(just so I don't forget all this until Monday): but only per annotated line, right? So if we don't merge the line, we need to still split out the { from the void f() { line (imagine the f() having a long enough name that the 3 annotated lines don't fit into one line any more, or a second statement after the f();</div>
<div><div class="h5">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr"> </p>
<p dir="ltr">However, I am not really saying that that is a better solution.. At any rate, it is low priority so let's discuss this on Monday.. </p><div><div>
<div class="gmail_quote">On Nov 21, 2013 4:11 PM, "Manuel Klimek" <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">So, in general, I think the problem is that we still need to break in the "fits into one line" case in some styles:<div>void f () {</div><div> f();</div><div>}</div><div>Here if we have a style that requires the { to go on the next line, we'd still generate at least 3 unwrapped lines; thus, even if "void f() {" fits into one line, we need to put the break before the { in somewhere, unless all of "void f() { f(); }" fits into one line and we can join the lines...</div>
<div><br></div><div>Where would you propose to do that?</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Nov 21, 2013 at 4:04 PM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>On Thu, Nov 21, 2013 at 3:56 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">I don't feel strongly about this. However, putting then decision into mustBreak would just work. You don't need any special casing for the single-line case as that is handled by a different code path..</p>
</blockquote><div><br></div></div><div>I don't understand that yet - if mustBreak retruns true, we'd introduce a break; when we join lines, we'd need to remove that break again, and count the right number of spaces, right?</div>
<div><div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr"> </p><div><div>
<div class="gmail_quote">On Nov 21, 2013 2:22 AM, "Alexander Kornienko" <<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Having played with this a bit, I found a few problems with not putting the braces into separate lines in the UnwrappedLinesParser:<div> * if we have braces on the same unwrapped line, we'll need to introduce a break when laying them out (using TokenAnnotator::mustBreak), and we'll have to undo this break when joining lines (IIUC, line joiner currently doesn't support this);</div>
<div> * when MustBreakBefore is set, we also make TotalLength > ColumnLimit, and we'll need to undo this in line joiner, which will also add complexity.</div><div><br></div><div>Overall, always having the braces on the same unwrapped line doesn't seem to be able to simplify the code =\<br>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Nov 20, 2013 at 7:32 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Wed, Nov 20, 2013 at 9:28 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>On Wed, Nov 20, 2013 at 6:12 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
@@ -681,6 +681,7 @@ void UnwrappedLineParser::parseStructura<br>
Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup ||<br>
Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
addUnwrappedLine();<br></blockquote><div><br></div></div></div><div>Does it still make sense to report the "{" as its own unwrapped line? Seems a bit convoluted to first report multiple lines and then merge them afterwards. I think this would make the merging code simpler.</div>
</div></div></div></blockquote><div><br></div></div><div>It also seemed strange to me. Should we instead handle BreakBeforeBraces in TokenAnnotator? This will require adding TokenType values for braces starting namespaces, classes/structs and, probably, enums. I can play with this a bit, it you think it makes sense.</div>
</div></div></div></blockquote><div><br></div></div></div><div>I might have already done this for enums. I don't think it is essential to add token types for all of these as e.g. enums and namespaces are really easy to detect. But adding token types might be the cleaner solution. I think that this makes sense but I remember having some kind of debate over this with Manuel, so he might have an opinion. </div>
</div><br></div></div>
</blockquote></div><br>
</div></div></div>
</blockquote></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>
</blockquote></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>