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

Daniel Jasper djasper at google.com
Wed Jul 31 16:24:03 PDT 2013


I have extended your patch and submitted it as r187540.

Cheers,
Daniel


On Mon, Jul 29, 2013 at 3:08 AM, Daniel Jasper <djasper at google.com> wrote:

>
>
>
> 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/20130731/9030dc23/attachment.html>


More information about the cfe-commits mailing list