[PATCH] D151047: [clang-format] Fix indent for selective formatting.

[Sedenion via Phabricator via cfe-commits]

Sedeniono marked 6 inline comments as done.
Sedeniono added inline comments.


================
Comment at: clang/unittests/Format/FormatTestSelective.cpp:641-646
+  Style = getLLVMStyle();
+  Style.FixNamespaceComments = false;
+  Code = "namespace ns {\n"
+         "#define REF(alias) alias alias_var;\n"
+         "}";
+  EXPECT_EQ(Code, format(Code, 51, 1));
----------------
owenpan wrote:
> Sedeniono wrote:
> > owenpan wrote:
> > > I suppose this would make the purpose of the test more clear. However, this new test (in either version) would pass without the patch, so it doesn't capture the bug mentioned in D151047#4369742?
> > When I originally submitted the fix for the incorrect selective formatting and you committed it to the main branch, some clang rename unit tests failed because they ran into some `assert()` in `LevelIndentTracker` (see [this post](https://reviews.llvm.org/D151047#4366917)), making a revert necessary. There was no test in the format Google Tests themselves that caught that mistake. Hence I added this test. But this also means that this test passes with and without the current patch. See an [earlier comment](https://reviews.llvm.org/D151047#4369742) where I describe the problem in more details.
> > 
> > Note that the change in `LineJoiner::join()` (which triggered the problem) is no longer in the current version of the patch because of some other unrelated changes that happened in the main branch since then. Again, please compare [another earlier comment](https://reviews.llvm.org/D151047#4396727).
> Then what do you think about changing the test case as suggested?
I now added the hyperlink. Also, added the "format this line only" comment, but on the `"}"` line, since that is the line that gets formatted. Also changed the length parameter to `format()` from 1 to 0; this never made any sense to me (a range of 0 length?), but it is done everywhere else, so whatever. Removing the `Style.FixNamespaceComments = false;` line would be wrong because the value is `true` for the LLVM style, so I kept it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151047