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

[Manuel Klimek]

After talking with Alex about what's needed to be done here, I convinced
myself of the opposite :)

The problem is that it looks to me like we don't win anything:
- if we have the break inside one annotated line with MustBreakBefore, that
means we have to add logic to the joiner to be able to remove a break
inside the annotated line (currently we only merge if annotated lines are
not broken inside, which allows us to only look at the split points).I
think adding the { joining to the line joining logic is not better than
having it in the annotated lines merging logic (like it currently is).

- if we always keep { on the same line in the layouter, no matter what the
style is, we make the joiner simpler, but then we have to add a step at the
same part of the pipeline as the joiner to split the {'s out depending on
the style; I think this makes the code more complex than Alex' current
suggestion





On Thu, Nov 21, 2013 at 9:56 AM, Manuel Klimek <klimek at google.com> wrote:

> 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.
>>
>
> I think the main motivation was that we didn't need it at the point, and
> we didn't have all the nuts and bolts in place to transfer the information
> precisely enough to the layouter...
>
> Now seems to be a good time to change this... :)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131121/1e245640/attachment.html>