[PATCH] D45719: [clang-Format] Fix indentation of member call after block

[Anders Karlsson via Phabricator via cfe-commits]

ank added inline comments.


================
Comment at: unittests/Format/FormatTest.cpp:4359
+               "    return 3;\n"
+               "  }).as("");\n"
+               "}");
----------------
klimek wrote:
> ank wrote:
> > klimek wrote:
> > > ank wrote:
> > > > klimek wrote:
> > > > > What would be interesting is tests that:
> > > > > a) have another value after the closing }; doesn't really make sense in this test, but usually these are in calls
> > > > > f([]() { ... }, foo, bar).call(...)
> > > > > 
> > > > > b) make .as("") have paramters that go beyond the limit
> > > > > 
> > > > > c) add another chained call behind .as(""). 
> > > > Hello, sorry for the late follow up on this! 
> > > > 
> > > > Indeed an interesting thing to test, and so I did, the patterns you describe seems to give strange indentation as far back as release_36 and probably has always done so. The case I'm testing here worked in release_40 but stopped working somewhere before release_50
> > > > 
> > > > maybe fixing those other cases can be done in another patch
> > > > 
> > > > cheers,
> > > > Anders
> > > I meant: please add these tests :)
> > I need to clarify, those tests will not be correctly indented by this commit, I could add those tests but then they would verify a faulty behaviour, this is how they will be indented even with this patch applied:
> > 
> > 
> > ```
> >     // Dont break if only closing statements before member call
> >     verifyFormat("test() {\n"
> >                  "  ([]() -> {\n"
> >                  "    int b = 32;\n"
> >                  "    return 3;\n"
> >                  "  }).foo();\n"
> >                  "}");
> >     verifyFormat("test() {\n"
> >                  "  (\n"
> >                  "      []() -> {\n"
> >                  "        int b = 32;\n"
> >                  "        return 3;\n"
> >                  "      },\n"
> >                  "      foo, bar)\n"
> >                  "      .foo();\n"
> >                  "}");
> >     verifyFormat("test() {\n"
> >                  "  ([]() -> {\n"
> >                  "    int b = 32;\n"
> >                  "    return 3;\n"
> >                  "  })\n"
> >                  "      .foo()\n"
> >                  "      .bar();\n"
> >                  "}");
> >     verifyFormat("test() {\n"
> >                  "  ([]() -> {\n"
> >                  "    int b = 32;\n"
> >                  "    return 3;\n"
> >                  "  })\n"
> >                  "      .foo(\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"\n"
> >                  "           \"bbbbbbbbbbbbbbbbbbbbbb\");\n"
> >                  "}");
> > 
> >     verifyFormat("test() {\n"
> >                  "  foo([]() -> {\n"
> >                  "    int b = 32;\n"
> >                  "    return 3;\n"
> >                  "  }).foo();\n"
> >                  "}");
> >     verifyFormat("test() {\n"
> >                  "  foo(\n"
> >                  "      []() -> {\n"
> >                  "        int b = 32;\n"
> >                  "        return 3;\n"
> >                  "      },\n"
> >                  "      foo, bar)\n"
> >                  "      .as();\n"
> >                  "}");
> >     verifyFormat("test() {\n"
> >                  "  foo([]() -> {\n"
> >                  "    int b = 32;\n"
> >                  "    return 3;\n"
> >                  "  })\n"
> >                  "      .foo()\n"
> >                  "      .bar();\n"
> >                  "}");
> >     verifyFormat("test() {\n"
> >                  "  foo([]() -> {\n"
> >                  "    int b = 32;\n"
> >                  "    return 3;\n"
> >                  "  })\n"
> >                  "      .as(\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"\n"
> >                  "          \"bbbbbbbbbbbbbbbbbbbbbb\");\n"
> >                  "}");
> > ```
> I'm not sure we agree that the behavior is "faulty" - I do believe it was an intentional change :)
> This is an indent of 4 from the start of the expression that call belongs to.
> For example, in example (2), having the .foo() at the end of line after a potentially complex parameter strongly de-emphasizes the parameter.
> In example (3), how would you want to layout a chain of calls?
I see! I think your argument makes perfect sense, I think example 3 should be as is, considering that it is consistent with the other behaviour :) thanks for taking the time to explain!


Repository:
  rC Clang

https://reviews.llvm.org/D45719