[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:1372
   LLVMStyle.DerivePointerAlignment = false;
+  LLVMStyle.DesignatedInitializerIndentWidth = 4;
   LLVMStyle.DisableFormat = false;
----------------
MyDeveloperDay wrote:
> so lets say someone is using an IndentWidth of 2 and now you introduce this as being 4 here as the default
> 
> Without them changing anything, all their DesignatedIntializer code will get refactored to a IndentWidth of 4 rather than the 2 it was previously
> 
> This is where we get called out for "changing the defaults", which really we are not we are just reclassifying how it behaves.
> 
> What we normally say here is can you point us to a public style that has an independent DesignatedIntializerIndentWidth which is independent from the levels of IndentWidth everywhere else.
> 
> Whilst I can see more knobs feels good, this will change code and we'll have to manage that expectation.
> 
> 
> 
Yep, so as per my comment in `clang/docs/ClangFormatStyleOptions.rst` I think I am changing my mind about this anyway.

My motivation for making this change is to support the [[ https://github.com/capnproto/capnproto/blob/master/kjdoc/style-guide.md | KJ style guide ]] which is quoted in the [[ https://github.com/llvm/llvm-project/issues/51070 | github issue ]] - I work on a team that uses the KJ style guide.

The KJ style guide wants a designated initializer indent width of 2 along with a "normal" indent width of 2, so there is no explicit need for us to have those two values be different.

When originally making these changes, I did think that having "more knobs" was a good idea, but I agree that this could lead to annoying behaviour for some users and they would probably expect the designated initializer indent to match either the normal indent or the continuation indent.

How about I change the option to an integer and, when it's -1 (the default), the designated initializer indent matches the continuation indent, but if it is set to a value >= 0 then that number of columns is used instead? 



================
Comment at: clang/unittests/Format/FormatTest.cpp:4828
+               "    .yyyyyyyyyyyyyyyyyy = 2,\n"
+               "    .zzzzzzzzzzzzzzzzzz = 3};\n",
+               Style);
----------------
MyDeveloperDay wrote:
> Can we fix the brace positioning too
What would you expect the brace positioning to be here? I noticed that, with this configuration at least, the closing brace goes at the end of the same line (as the last initializer) when there is no trailing comma but goes on a new line if there is a trailing comma.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146101/new/

https://reviews.llvm.org/D146101