r195256 - Added an option to allow short function bodies be placed on a single line.

[Daniel Jasper]

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 <klimek at google.com> wrote:

> On Thu, Nov 21, 2013 at 6:54 PM, Daniel Jasper <djasper at google.com> 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" <klimek at google.com> 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 <klimek at google.com>wrote:
>>>
>>>> On Thu, Nov 21, 2013 at 3:56 PM, Daniel Jasper <djasper at google.com>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" <alexfh at google.com>
>>>>> 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 <djasper at google.com>wrote:
>>>>>>
>>>>>>>
>>>>>>> On Wed, Nov 20, 2013 at 9:28 AM, Alexander Kornienko <
>>>>>>> alexfh at google.com> wrote:
>>>>>>>
>>>>>>>> On Wed, Nov 20, 2013 at 6:12 PM, Daniel Jasper <djasper at google.com>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.
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131121/b4f411ce/attachment.html>