[PATCH] [clang-format] Added: FormatStyle::IndentNamespaces to indent the content of namespaces

[Daniel Jasper]

On Mon, Jul 29, 2013 at 12:00 PM, Curdeius Curdeius <curdeius at gmail.com>wrote:

> Hi,
> thanks for your comments.
> I've fixed the patch.
> File formatting should be ok now.
> I've changed the name to IndentInNamespaces as suggested.
>

Thank you. Do you have commit access or should I commit this for you?

What about the indenting of inner namespaces: take a look for instance at
> WebKit coding style about namespaces:
> http://www.webkit.org/coding/coding-style.html#indentation-namespace.
> It's not exactly what I proposed, but it's close still.
>

I will implement what is required for WebKit. Lets see how we go from there.

Cheers,
Daniel

Cheers,
> Marek Kurdej
>
>
> 2013/7/29 Daniel Jasper <djasper at google.com>
>
>> Please attach patches as plain text, not as zip-files. Or alternatively,
>> use phabricator (http://llvm-reviews.chandlerc.com/).
>>
>> Comments inline.
>>
>> +  /// \brief Indent the content of namespaces by one level.
>>> +  ///
>>> +  /// When false, use the same indentation level as outside block (file
>>> or namespace).
>>>
>>
>> Please adhere to the 80 column limit. Or ideally, just use clang-format
>> ;-).
>>
>>
>>> +  bool IndentNamespaces;
>>>
>>
>> IndentInNamespaces might be slightly more precise, but I am not sure.
>>
>>
>>> -    parseBlock(/*MustBeDeclaration=*/true, 0);
>>> +    parseBlock(/*MustBeDeclaration=*/true,
>>> /*AddLevels=*/Style.IndentNamespaces);
>>>
>>
>> Please adhere to the 80 column limit.
>>
>>
>>> +  FormatStyle getLLVMStyleWithIndentNamespaces(bool IndentNamespaces =
>>> true) {
>>> +    FormatStyle Style = getLLVMStyle();
>>> +    Style.IndentNamespaces = IndentNamespaces;
>>> +    return Style;
>>> +  }
>>> +
>>> +  FormatStyle getGoogleStyleWithIndentNamespaces(bool IndentNamespaces
>>> = true) {
>>> +    FormatStyle Style = getGoogleStyle();
>>> +    Style.IndentNamespaces = IndentNamespaces;
>>> +    return Style;
>>> +  }
>>>
>>
>> I don't think it is worth adding these methods. Just define a local style
>> in your test:
>>
>> TEST_F(..., ...) {
>>   FormatStyle Style = getLLVMStyle();
>>   Style.IndentNamespace = true;
>>   ...
>> }
>>
>>
>>> On Fri, Jul 26, 2013 at 2:17 PM, Curdeius Curdeius <curdeius at gmail.com>wrote:
>>> Hi,
>>> Clang-format was lacking an option to add indentation within namespaces.
>>> I've created a patch that adds this option to clang::Format::FormatStyle.
>>> There is also a basic test.
>>> You'll find it in the attachment.
>>> I think it will be nice to have a possibility to indent only the content
>>> of namespaces, but excluding the inner namespaces, i.e. to have something
>>> like:
>>>
>>>   class Indented1;
>>> namespace inner1 {
>>>   class Indented2;
>>> } // namespace inner1
>>> namespace inner2 {
>>> } // namespace inner2
>>> } // namespace outer
>>> instead of:
>>>
>>>   class Indented1;
>>>   namespace inner1 {
>>>     class Indented2;
>>>   } // namespace inner1
>>>   namespace inner2 {
>>>   } // namespace inner2
>>> } // namespace outer
>>>
>>
>> Really? I have never seen the latter before and it would confuse me quite
>> a bit.
>>
>> Cheers,
>> Daniel
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130729/325826e2/attachment.html>