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.
On Thu, Nov 21, 2013 at 10:47 AM, Manuel Klimek <[email protected]> wrote: > On Thu, Nov 21, 2013 at 6:54 PM, Daniel Jasper <[email protected]> wrote: > >> In ContinuationIndenter::mustBreak().. clang-format does not use that >> code at all if everything fits on one line.. >> > (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(); > > >> 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.. >> On Nov 21, 2013 4:11 PM, "Manuel Klimek" <[email protected]> wrote: >> >>> So, in general, I think the problem is that we still need to break in >>> the "fits into one line" case in some styles: >>> void f () { >>> f(); >>> } >>> 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... >>> >>> Where would you propose to do that? >>> >>> >>> On Thu, Nov 21, 2013 at 4:04 PM, Manuel Klimek <[email protected]>wrote: >>> >>>> On Thu, Nov 21, 2013 at 3:56 PM, Daniel Jasper <[email protected]>wrote: >>>> >>>>> 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.. >>>>> >>>> >>>> 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? >>>> >>>> >>>>> On Nov 21, 2013 2:22 AM, "Alexander Kornienko" <[email protected]> >>>>> wrote: >>>>> >>>>>> Having played with this a bit, I found a few problems with not >>>>>> putting the braces into separate lines in the UnwrappedLinesParser: >>>>>> * 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); >>>>>> * when MustBreakBefore is set, we also make TotalLength > >>>>>> ColumnLimit, and we'll need to undo this in line joiner, which will also >>>>>> add complexity. >>>>>> >>>>>> Overall, always having the braces on the same unwrapped line doesn't >>>>>> seem to be able to simplify the code =\ >>>>>> >>>>>> >>>>>> On Wed, Nov 20, 2013 at 7:32 PM, Daniel Jasper <[email protected]>wrote: >>>>>> >>>>>>> >>>>>>> On Wed, Nov 20, 2013 at 9:28 AM, Alexander Kornienko < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> On Wed, Nov 20, 2013 at 6:12 PM, Daniel Jasper >>>>>>>> <[email protected]>wrote: >>>>>>>> >>>>>>>>> >>>>>>>>>> @@ -681,6 +681,7 @@ void UnwrappedLineParser::parseStructura >>>>>>>>>> Style.BreakBeforeBraces == >>>>>>>>>> FormatStyle::BS_Stroustrup || >>>>>>>>>> Style.BreakBeforeBraces == FormatStyle::BS_Allman) >>>>>>>>>> addUnwrappedLine(); >>>>>>>>>> >>>>>>>>> >>>>>>>>> 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. >>>>>>>>> >>>>>>>> >>>>>>>> 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. >>>>>>>> >>>>>>> >>>>>>> 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. >>>>>>> >>>>>>> >>>>>> >>>> >>> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
