[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe8111502d869: [clang-format] use spaces for alignment with 
UT_ForContinuationAndIndentation (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10151,17 +10151,18 @@
" \t \tcomment */",
Tab));
   EXPECT_EQ("{\n"
-"  /*\n"
-"   * Comment\n"
-"   */\n"
-"  int i;\n"
+"/*\n"
+" * Comment\n"
+" */\n"
+"int i;\n"
 "}",
 format("{\n"
"\t/*\n"
"\t * Comment\n"
"\t */\n"
"\t int i;\n"
-   "}"));
+   "}",
+   Tab));
 
   Tab.UseTab = FormatStyle::UT_ForContinuationAndIndentation;
   Tab.TabWidth = 8;
@@ -10332,15 +10333,245 @@
"\t*/\n"
"}",
Tab));
+  EXPECT_EQ("/* some\n"
+"   comment */",
+format(" \t \t /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"   comment */",
+format(" \t \t int a; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"comment */",
+format(" \t \t int\ta; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("f(\"\t\t\"); /* some\n"
+"comment */",
+format(" \t \t f(\"\t\t\"); /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("{\n"
+"\t/*\n"
+"\t * Comment\n"
+"\t */\n"
+"\tint i;\n"
+"}",
+format("{\n"
+   "\t/*\n"
+   "\t * Comment\n"
+   "\t */\n"
+   "\t int i;\n"
+   "}",
+   Tab));
+  Tab.TabWidth = 2;
+  Tab.IndentWidth = 2;
+  EXPECT_EQ("{\n"
+"\t/* \n"
+"\t\t  */\n"
+"}",
+format("{\n"
+   "/* \n"
+   "\t  */\n"
+   "}",
+   Tab));
+  EXPECT_EQ("{\n"
+"\t/*\n"
+"\t\taa\n"
+"\t\tb\n"
+"\t*/\n"
+"}",
+format("{\n"
+   "/*\n"
+   "\taa b\n"
+   "*/\n"
+   "}",
+   Tab));
+  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveDeclarations = true;
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 4;
+  verifyFormat("class Assign {\n"
+   "\tvoid f() {\n"
+   "\t\tint x  = 123;\n"
+   "\t\tint random = 4;\n"
+   "\t\tstd::string alphabet =\n"
+   "\t\t\t\"abcdefghijklmnopqrstuvwxyz\";\n"
+   "\t}\n"
+   "};",
+   Tab);
+
+  Tab.UseTab = FormatStyle::UT_AlignWithSpaces;
+  Tab.TabWidth = 8;
+  Tab.IndentWidth = 8;
+  EXPECT_EQ("if ( && // q\n"
+"bb) // w\n"
+"\t;",
+format("if ( &&// q\n"
+   "bb)// w\n"
+   ";",
+   Tab));
+  EXPECT_EQ("if (aaa && bbb) // w\n"
+"\t;",
+format("if(aaa&)// w\n"
+   ";",
+   Tab));
+  verifyFormat("class X {\n"
+   "\tvoid f() {\n"
+   "\t\tsomeFunction(parameter1,\n"
+   "\t\t parameter2);\n"
+   "\t}\n"
+   "};",
+   Tab);
+  verifyFormat("#define A\\\n"
+   "\tvoid f() {   \\\n"
+   "\t\tsomeFunction(\\\n"
+   "\t\tparameter1,  \\\n"
+   "\t\tparameter2); \\\n"
+   "\t}",
+   Tab);
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 8;
+  verifyFormat("class TabWidth4Indent8 {\n"
+   

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert updated this revision to Diff 256938.
fickert added a comment.

rebased, fixed two unit tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10133,17 +10133,18 @@
" \t \tcomment */",
Tab));
   EXPECT_EQ("{\n"
-"  /*\n"
-"   * Comment\n"
-"   */\n"
-"  int i;\n"
+"/*\n"
+" * Comment\n"
+" */\n"
+"int i;\n"
 "}",
 format("{\n"
"\t/*\n"
"\t * Comment\n"
"\t */\n"
"\t int i;\n"
-   "}"));
+   "}",
+   Tab));
 
   Tab.UseTab = FormatStyle::UT_ForContinuationAndIndentation;
   Tab.TabWidth = 8;
@@ -10314,15 +10315,245 @@
"\t*/\n"
"}",
Tab));
+  EXPECT_EQ("/* some\n"
+"   comment */",
+format(" \t \t /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"   comment */",
+format(" \t \t int a; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"comment */",
+format(" \t \t int\ta; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("f(\"\t\t\"); /* some\n"
+"comment */",
+format(" \t \t f(\"\t\t\"); /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("{\n"
+"\t/*\n"
+"\t * Comment\n"
+"\t */\n"
+"\tint i;\n"
+"}",
+format("{\n"
+   "\t/*\n"
+   "\t * Comment\n"
+   "\t */\n"
+   "\t int i;\n"
+   "}",
+   Tab));
+  Tab.TabWidth = 2;
+  Tab.IndentWidth = 2;
+  EXPECT_EQ("{\n"
+"\t/* \n"
+"\t\t  */\n"
+"}",
+format("{\n"
+   "/* \n"
+   "\t  */\n"
+   "}",
+   Tab));
+  EXPECT_EQ("{\n"
+"\t/*\n"
+"\t\taa\n"
+"\t\tb\n"
+"\t*/\n"
+"}",
+format("{\n"
+   "/*\n"
+   "\taa b\n"
+   "*/\n"
+   "}",
+   Tab));
+  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveDeclarations = true;
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 4;
+  verifyFormat("class Assign {\n"
+   "\tvoid f() {\n"
+   "\t\tint x  = 123;\n"
+   "\t\tint random = 4;\n"
+   "\t\tstd::string alphabet =\n"
+   "\t\t\t\"abcdefghijklmnopqrstuvwxyz\";\n"
+   "\t}\n"
+   "};",
+   Tab);
+
+  Tab.UseTab = FormatStyle::UT_AlignWithSpaces;
+  Tab.TabWidth = 8;
+  Tab.IndentWidth = 8;
+  EXPECT_EQ("if ( && // q\n"
+"bb) // w\n"
+"\t;",
+format("if ( &&// q\n"
+   "bb)// w\n"
+   ";",
+   Tab));
+  EXPECT_EQ("if (aaa && bbb) // w\n"
+"\t;",
+format("if(aaa&)// w\n"
+   ";",
+   Tab));
+  verifyFormat("class X {\n"
+   "\tvoid f() {\n"
+   "\t\tsomeFunction(parameter1,\n"
+   "\t\t parameter2);\n"
+   "\t}\n"
+   "};",
+   Tab);
+  verifyFormat("#define A\\\n"
+   "\tvoid f() {   \\\n"
+   "\t\tsomeFunction(\\\n"
+   "\t\tparameter1,  \\\n"
+   "\t\tparameter2); \\\n"
+   "\t}",
+   Tab);
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 8;
+  verifyFormat("class TabWidth4Indent8 {\n"
+   "\t\tvoid f() {\n"
+   "\t\t\t\tsomeFunction(parameter1,\n"
+   "\t\t\t\t

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added a comment.

Great, thank you!

I do not have push access to the repository, so someone else would need to 
commit this for me:  Maximilian Fickert 

Should I add the fix for the test cases missing the tab configuration (see my 
last inline comment: https://reviews.llvm.org/D75034#inline-703423)? Or submit 
it as a separate change set?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

This LGTM, thank you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert updated this revision to Diff 253349.
fickert added a comment.

Added documentation and missing test cases for 0-width tabs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10314,15 +10314,244 @@
"\t*/\n"
"}",
Tab));
+  EXPECT_EQ("/* some\n"
+"   comment */",
+format(" \t \t /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"   comment */",
+format(" \t \t int a; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"comment */",
+format(" \t \t int\ta; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("f(\"\t\t\"); /* some\n"
+"comment */",
+format(" \t \t f(\"\t\t\"); /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("{\n"
+"  /*\n"
+"   * Comment\n"
+"   */\n"
+"  int i;\n"
+"}",
+format("{\n"
+   "\t/*\n"
+   "\t * Comment\n"
+   "\t */\n"
+   "\t int i;\n"
+   "}"));
+  Tab.TabWidth = 2;
+  Tab.IndentWidth = 2;
+  EXPECT_EQ("{\n"
+"\t/* \n"
+"\t\t  */\n"
+"}",
+format("{\n"
+   "/* \n"
+   "\t  */\n"
+   "}",
+   Tab));
+  EXPECT_EQ("{\n"
+"\t/*\n"
+"\t\taa\n"
+"\t\tb\n"
+"\t*/\n"
+"}",
+format("{\n"
+   "/*\n"
+   "\taa b\n"
+   "*/\n"
+   "}",
+   Tab));
+  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveDeclarations = true;
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 4;
+  verifyFormat("class Assign {\n"
+   "\tvoid f() {\n"
+   "\t\tint x  = 123;\n"
+   "\t\tint random = 4;\n"
+   "\t\tstd::string alphabet =\n"
+   "\t\t\t\"abcdefghijklmnopqrstuvwxyz\";\n"
+   "\t}\n"
+   "};",
+   Tab);
+
+  Tab.UseTab = FormatStyle::UT_AlignWithSpaces;
+  Tab.TabWidth = 8;
+  Tab.IndentWidth = 8;
+  EXPECT_EQ("if ( && // q\n"
+"bb) // w\n"
+"\t;",
+format("if ( &&// q\n"
+   "bb)// w\n"
+   ";",
+   Tab));
+  EXPECT_EQ("if (aaa && bbb) // w\n"
+"\t;",
+format("if(aaa&)// w\n"
+   ";",
+   Tab));
+  verifyFormat("class X {\n"
+   "\tvoid f() {\n"
+   "\t\tsomeFunction(parameter1,\n"
+   "\t\t parameter2);\n"
+   "\t}\n"
+   "};",
+   Tab);
+  verifyFormat("#define A\\\n"
+   "\tvoid f() {   \\\n"
+   "\t\tsomeFunction(\\\n"
+   "\t\tparameter1,  \\\n"
+   "\t\tparameter2); \\\n"
+   "\t}",
+   Tab);
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 8;
+  verifyFormat("class TabWidth4Indent8 {\n"
+   "\t\tvoid f() {\n"
+   "\t\t\t\tsomeFunction(parameter1,\n"
+   "\t\t\t\t parameter2);\n"
+   "\t\t}\n"
+   "};",
+   Tab);
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 4;
+  verifyFormat("class TabWidth4Indent4 {\n"
+   "\tvoid f() {\n"
+   "\t\tsomeFunction(parameter1,\n"
+   "\t\t parameter2);\n"
+   "\t}\n"
+   "};",
+   Tab);
+  Tab.TabWidth = 8;
+  Tab.IndentWidth = 4;
+  verifyFormat("class TabWidth8Indent4 {\n"
+   "void f() {\n"
+   "\tsomeFunction(parameter1,\n"
+   "\t parameter2);\n"
+   "}\n"
+   "};",
+   Tab);

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert updated this revision to Diff 253327.
fickert added a comment.

I moved the changed behavior to a new option (`UT_AlignWithSpaces`), and added 
corresponding unit tests by copying and adapting the tests for 
`UT_ForContinuationAndIndentation`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10314,15 +10314,244 @@
"\t*/\n"
"}",
Tab));
+  EXPECT_EQ("/* some\n"
+"   comment */",
+format(" \t \t /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"   comment */",
+format(" \t \t int a; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"comment */",
+format(" \t \t int\ta; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("f(\"\t\t\"); /* some\n"
+"comment */",
+format(" \t \t f(\"\t\t\"); /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("{\n"
+"  /*\n"
+"   * Comment\n"
+"   */\n"
+"  int i;\n"
+"}",
+format("{\n"
+   "\t/*\n"
+   "\t * Comment\n"
+   "\t */\n"
+   "\t int i;\n"
+   "}"));
+  Tab.TabWidth = 2;
+  Tab.IndentWidth = 2;
+  EXPECT_EQ("{\n"
+"\t/* \n"
+"\t\t  */\n"
+"}",
+format("{\n"
+   "/* \n"
+   "\t  */\n"
+   "}",
+   Tab));
+  EXPECT_EQ("{\n"
+"\t/*\n"
+"\t\taa\n"
+"\t\tb\n"
+"\t*/\n"
+"}",
+format("{\n"
+   "/*\n"
+   "\taa b\n"
+   "*/\n"
+   "}",
+   Tab));
+  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveDeclarations = true;
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 4;
+  verifyFormat("class Assign {\n"
+   "\tvoid f() {\n"
+   "\t\tint x  = 123;\n"
+   "\t\tint random = 4;\n"
+   "\t\tstd::string alphabet =\n"
+   "\t\t\t\"abcdefghijklmnopqrstuvwxyz\";\n"
+   "\t}\n"
+   "};",
+   Tab);
+
+  Tab.UseTab = FormatStyle::UT_AlignWithSpaces;
+  Tab.TabWidth = 8;
+  Tab.IndentWidth = 8;
+  EXPECT_EQ("if ( && // q\n"
+"bb) // w\n"
+"\t;",
+format("if ( &&// q\n"
+   "bb)// w\n"
+   ";",
+   Tab));
+  EXPECT_EQ("if (aaa && bbb) // w\n"
+"\t;",
+format("if(aaa&)// w\n"
+   ";",
+   Tab));
+  verifyFormat("class X {\n"
+   "\tvoid f() {\n"
+   "\t\tsomeFunction(parameter1,\n"
+   "\t\t parameter2);\n"
+   "\t}\n"
+   "};",
+   Tab);
+  verifyFormat("#define A\\\n"
+   "\tvoid f() {   \\\n"
+   "\t\tsomeFunction(\\\n"
+   "\t\tparameter1,  \\\n"
+   "\t\tparameter2); \\\n"
+   "\t}",
+   Tab);
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 8;
+  verifyFormat("class TabWidth4Indent8 {\n"
+   "\t\tvoid f() {\n"
+   "\t\t\t\tsomeFunction(parameter1,\n"
+   "\t\t\t\t parameter2);\n"
+   "\t\t}\n"
+   "};",
+   Tab);
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 4;
+  verifyFormat("class TabWidth4Indent4 {\n"
+   "\tvoid f() {\n"
+   "\t\tsomeFunction(parameter1,\n"
+   "\t\t parameter2);\n"
+   "\t}\n"
+   "};",
+   Tab);
+  Tab.TabWidth = 8;
+  Tab.IndentWidth = 4;
+  verifyFormat("class TabWidth8Indent4 {\n"
+   "void f() {\n"
+   "\tsomeFunction(parameter1,\n"
+   "\t 

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:10146
"\t int i;\n"
"}"));
 

This test seems to be missing the tab configuration (`Tab`) as an argument to 
`format`, and the same test with missing `Tab` argument appears again in line 
10337. Adding the `Tab` argument will also change the expected result. I think 
the test case here should look like this:

```
  EXPECT_EQ("{\n"
"/*\n"
" * Comment\n"
" */\n"
"int i;\n"
"}",
format("{\n"
   "\t/*\n"
   "\t * Comment\n"
   "\t */\n"
   "\t int i;\n"
   "}",
   Tab));
```

and a similar change should be made to the one in line 10337.

I assume this should be fixed in a separate commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added a comment.

In D75034#1938626 , @MyDeveloperDay 
wrote:

> is there overlap here D76197: clang-format: Use block indentation for braced 
> initializations 


I think these changes are unrelated.


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I want to believe that it was a mistake on the original developers part, but I 
just can't tell.

A search of github shows people using this in their repos

https://github.com/search?q=ForContinuationAndIndentation=Code

I almost feel unless the original author can comment we are at an impasse, 
maybe we have to add:

`ForAllContinuationAndIndentation`

And if at some point in the future we can collapse both options into 1 then 
that would be better, but I just don't feel qualified to say change everyone 
when I don't use that option myself because I don't use tabs.

Whatever you add need to be heavily document that we mean everywhere, and if 
you ever find any cases where its doesn't work we intend for them to be 
changed. i.e. be very explicit in the review/commit comments


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

is there overlap here D76197: clang-format: Use block indentation for braced 
initializations 


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added a comment.

From the original commit, it does seem that this behavior was desired.

However, from the name of the option and description in the documentation ("Use 
tabs only for line continuation and indentation"), I would expect it to use 
tabs only for indentation and continuation, but spaces for alignment. The 
current behavior uses spaces for some types of alignment that appear in the 
middle of a line (e.g. consecutive assignments/declarations), but not for 
others that occur at the beginning of a line (e.g. function parameters). The 
bug report seems to indicate that other people also expected a different 
behavior given the description.

Does anyone else want to comment? Otherwise I can move the changed behavior 
into a new option; in that case, I'd be happy about naming suggestions.


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a subscriber: mxbOctasic.
mitchell-stellar added a comment.

Digging through the history, it seems this behavior was explicitly desired: 
https://reviews.llvm.org/D19028. @mxbOctasic was the original author and 
perhaps could comment. Regardless, I think there should be a new option.


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I tend to agree with your assessment, I think its worth waiting for some others 
to comment.




Comment at: clang/unittests/Format/FormatTest.cpp:10293
Tab));
-  EXPECT_EQ("/*\n"
-"\t  a\t\tcomment\n"

fickert wrote:
> MyDeveloperDay wrote:
> > Sorry this isn't clear to be which test this is a duplicate of, as this is 
> > an example where there are tabs in the comment and trailing tabs
> This test case is identical to the one in line 10182-10190. There are several 
> test cases that appear multiple times, but usually with changes to the 
> formatting settings in between. This is not the case for these two test cases 
> (no changes in between), so this one seems to be obsolete.
thank you for highlighting that.


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added a comment.

In D75034#1889107 , @MyDeveloperDay 
wrote:

> > Is this intentional, or is it an oversight?
>
> This is the key question, which I'm not sure how we can answer without the 
> help of the original authors @djasper and @klimek
>
> For example I can't understand with the example below if the TabWith is 8 why 
> are you putting 13 spaces in, rather than the original test which had 1 tab 
> (assuming 8 characters) and 5 spaces? (to make up 13)
>
> Could you explain why this is the correct change for this example why its 
> correct to put 13 spaces? (is this not continuation or indentation?)
>
>   Tab.UseTab = FormatStyle::UT_ForContinuationAndIndentation;
>   Tab.TabWidth = 8;
>   Tab.IndentWidth = 8;
>   verifyFormat("class X {\n"
>"\tvoid f() {\n"
>"\t\tsomeFunction(parameter1,\n"
>"\t\t.parameter2);\n"
>"\t}\n"
>"};",
>Tab);
>   


In this example, the first two tabs are for indentation, and the remaining 
whitespace before parameter2 is for alignment (not continuation) with 
parameter1. If tabs are used in the alignment, the formatted code will only 
look aligned with the correct tab width, whereas if the alignment is done with 
spaces, the code will look aligned with arbitrary tab width.

I believe this to be the expected behavior for 
`UT_ForContinuationAndIndentation`, and tabs should only be used for alignment 
with `UT_Always`, but I would also be happy about comments from the original 
authors.


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert marked an inline comment as done.
fickert added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:10293
Tab));
-  EXPECT_EQ("/*\n"
-"\t  a\t\tcomment\n"

MyDeveloperDay wrote:
> Sorry this isn't clear to be which test this is a duplicate of, as this is an 
> example where there are tabs in the comment and trailing tabs
This test case is identical to the one in line 10182-10190. There are several 
test cases that appear multiple times, but usually with changes to the 
formatting settings in between. This is not the case for these two test cases 
(no changes in between), so this one seems to be obsolete.


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added subscribers: djasper, klimek.
MyDeveloperDay added a comment.

> Is this intentional, or is it an oversight?

This is the key question, which I'm not sure how we can answer without the help 
of the original authors @djasper and @klimek

For example I can't understand with the example below if the TabWith is 8 why 
are you putting 13 spaces in, rather than the original test which had 1 tab 
(assuming 8 characters) and 5 spaces? (to make up 13)

Could you explain why this is the correct change for this example why its 
correct to put 13 spaces? (is this not continuation or indentation?)

  Tab.UseTab = FormatStyle::UT_ForContinuationAndIndentation;
  Tab.TabWidth = 8;
  Tab.IndentWidth = 8;
  verifyFormat("class X {\n"
   "\tvoid f() {\n"
   "\t\tsomeFunction(parameter1,\n"
   "\t\t.parameter2);\n"
   "\t}\n"
   "};",
   Tab);


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:10293
Tab));
-  EXPECT_EQ("/*\n"
-"\t  a\t\tcomment\n"

Sorry this isn't clear to be which test this is a duplicate of, as this is an 
example where there are tabs in the comment and trailing tabs


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added a comment.

I understand the caution around changing the unit tests. The tests I modified 
assumed that alignment would be made with mixed tabs and spaces, which is 
arguably not the expected behavior with `UT_ForContinuationAndIndentation` (see 
https://bugs.llvm.org/show_bug.cgi?id=38381).  I removed a test case that was a 
duplicate (the one in line 10293 is identical to the one in line 10182, and 
there are no changes to the formatting configuration in between). I added two 
test cases to ensure that the behavior is consistent for alignment in 
multi-line comments, e.g. the formatted code should be

  {
  \t/* 
  \t   
  }

instead of

  {
  \t/* 
  \t\t 
  }

when using `UT_ForContinuationAndIndentation` and a tab width of 2.


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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert updated this revision to Diff 246176.
fickert marked an inline comment as done.
fickert added a comment.

uploaded full context diff


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

https://reviews.llvm.org/D75034

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10139,7 +10139,7 @@
   verifyFormat("class X {\n"
"\tvoid f() {\n"
"\t\tsomeFunction(parameter1,\n"
-   "\t\t\t parameter2);\n"
+   "\t\t parameter2);\n"
"\t}\n"
"};",
Tab);
@@ -10155,7 +10155,7 @@
   verifyFormat("class TabWidth4Indent8 {\n"
"\t\tvoid f() {\n"
"\t\t\t\tsomeFunction(parameter1,\n"
-   "\t\t\t\t\t\t\t parameter2);\n"
+   "\t\t\t\t parameter2);\n"
"\t\t}\n"
"};",
Tab);
@@ -10164,7 +10164,7 @@
   verifyFormat("class TabWidth4Indent4 {\n"
"\tvoid f() {\n"
"\t\tsomeFunction(parameter1,\n"
-   "\t\t\t\t\t parameter2);\n"
+   "\t\t parameter2);\n"
"\t}\n"
"};",
Tab);
@@ -10173,15 +10173,15 @@
   verifyFormat("class TabWidth8Indent4 {\n"
"void f() {\n"
"\tsomeFunction(parameter1,\n"
-   "\t\t parameter2);\n"
+   "\t parameter2);\n"
"}\n"
"};",
Tab);
   Tab.TabWidth = 8;
   Tab.IndentWidth = 8;
   EXPECT_EQ("/*\n"
-"\t  a\t\tcomment\n"
-"\t  in multiple lines\n"
+"  a\t\tcomment\n"
+"  in multiple lines\n"
 "   */",
 format("   /*\t \t \n"
" \t \t a\t\tcomment\t \t\n"
@@ -10213,7 +10213,7 @@
   verifyFormat("class X {\n"
"\tvoid f() {\n"
"\t\tsomeFunction(parameter1,\n"
-   "\t\t\t parameter2);\n"
+   "\t\t parameter2);\n"
"\t}\n"
"};",
Tab);
@@ -10222,7 +10222,7 @@
"\t{\n"
"\t\tint a;\n"
"\t\tsomeFunction(,\n"
-   "\t\t\t\t bbb);\n"
+   "\t\t bbb);\n"
"\t},\n"
"\tp);\n"
"}",
@@ -10290,15 +10290,6 @@
"\t*/\n"
"}",
Tab));
-  EXPECT_EQ("/*\n"
-"\t  a\t\tcomment\n"
-"\t  in multiple lines\n"
-"   */",
-format("   /*\t \t \n"
-   " \t \t a\t\tcomment\t \t\n"
-   " \t \t in multiple lines\t\n"
-   " \t  */",
-   Tab));
   EXPECT_EQ("/* some\n"
 "   comment */",
 format(" \t \t /* some\n"
@@ -10331,6 +10322,29 @@
"\t */\n"
"\t int i;\n"
"}"));
+  Tab.TabWidth = 2;
+  Tab.IndentWidth = 2;
+  EXPECT_EQ("{\n"
+"\t/* \n"
+"\t    */\n"
+"}",
+format("{\n"
+   "/* \n"
+   "    */\n"
+   "}",
+   Tab));
+  EXPECT_EQ("{\n"
+"\t/*\n"
+"\t  aa\n"
+"\t  b\n"
+"\t*/\n"
+"}",
+format("{\n"
+   "/*\n"
+   "  aa b\n"
+   "*/\n"
+   "}",
+   Tab));
   Tab.AlignConsecutiveAssignments = true;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -49,7 +49,7 @@
   /// this replacement. It is needed for determining how \p Spaces is turned
   /// into tabs and spaces for some format styles.
   void replaceWhitespace(FormatToken , unsigned Newlines, unsigned Spaces,
- unsigned StartOfTokenColumn,
+ unsigned StartOfTokenColumn, bool isAligned = false,
  bool InPPDirective = false);
 
   /// Adds information about an unchangeable 

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

you need to add any diff as a full context diff  (normally by adding -U 
999) to the diff command


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

If you are changing unit tests then I'm nervous, imagine if this many tests 
change what the impact is on a large code base.  I think we need to understand 
more about why you are making this change and why you think its ok to 
change/remove existing tests which assert some behavior which up to now we have 
considered correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added a comment.

I added an `IsAligned` flag to `ParenState` and `Change` to track if an indent 
was created for alignment. I adapted the corresponding test cases where 
alignments were previously done with tabs, including a test case where leading 
whitespace in a comment is interpreted as alignment.

This is my first contribution to clang-format so I appreciate any suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert created this revision.
fickert added reviewers: clang-format, MyDeveloperDay, krasimir.
fickert added projects: clang-format, clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Use spaces instead of tabs for alignment with UT_ForContinuationAndIndentation 
to make the code aligned for any tab/indent width.

Fixes https://bugs.llvm.org/show_bug.cgi?id=38381


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75034

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10139,7 +10139,7 @@
   verifyFormat("class X {\n"
"\tvoid f() {\n"
"\t\tsomeFunction(parameter1,\n"
-   "\t\t\t parameter2);\n"
+   "\t\t parameter2);\n"
"\t}\n"
"};",
Tab);
@@ -10155,7 +10155,7 @@
   verifyFormat("class TabWidth4Indent8 {\n"
"\t\tvoid f() {\n"
"\t\t\t\tsomeFunction(parameter1,\n"
-   "\t\t\t\t\t\t\t parameter2);\n"
+   "\t\t\t\t parameter2);\n"
"\t\t}\n"
"};",
Tab);
@@ -10164,7 +10164,7 @@
   verifyFormat("class TabWidth4Indent4 {\n"
"\tvoid f() {\n"
"\t\tsomeFunction(parameter1,\n"
-   "\t\t\t\t\t parameter2);\n"
+   "\t\t parameter2);\n"
"\t}\n"
"};",
Tab);
@@ -10173,15 +10173,15 @@
   verifyFormat("class TabWidth8Indent4 {\n"
"void f() {\n"
"\tsomeFunction(parameter1,\n"
-   "\t\t parameter2);\n"
+   "\t parameter2);\n"
"}\n"
"};",
Tab);
   Tab.TabWidth = 8;
   Tab.IndentWidth = 8;
   EXPECT_EQ("/*\n"
-"\t  a\t\tcomment\n"
-"\t  in multiple lines\n"
+"  a\t\tcomment\n"
+"  in multiple lines\n"
 "   */",
 format("   /*\t \t \n"
" \t \t a\t\tcomment\t \t\n"
@@ -10213,7 +10213,7 @@
   verifyFormat("class X {\n"
"\tvoid f() {\n"
"\t\tsomeFunction(parameter1,\n"
-   "\t\t\t parameter2);\n"
+   "\t\t parameter2);\n"
"\t}\n"
"};",
Tab);
@@ -10222,7 +10222,7 @@
"\t{\n"
"\t\tint a;\n"
"\t\tsomeFunction(,\n"
-   "\t\t\t\t bbb);\n"
+   "\t\t bbb);\n"
"\t},\n"
"\tp);\n"
"}",
@@ -10290,15 +10290,6 @@
"\t*/\n"
"}",
Tab));
-  EXPECT_EQ("/*\n"
-"\t  a\t\tcomment\n"
-"\t  in multiple lines\n"
-"   */",
-format("   /*\t \t \n"
-   " \t \t a\t\tcomment\t \t\n"
-   " \t \t in multiple lines\t\n"
-   " \t  */",
-   Tab));
   EXPECT_EQ("/* some\n"
 "   comment */",
 format(" \t \t /* some\n"
@@ -10331,6 +10322,29 @@
"\t */\n"
"\t int i;\n"
"}"));
+  Tab.TabWidth = 2;
+  Tab.IndentWidth = 2;
+  EXPECT_EQ("{\n"
+"\t/* \n"
+"\t    */\n"
+"}",
+format("{\n"
+   "/* \n"
+   "    */\n"
+   "}",
+   Tab));
+  EXPECT_EQ("{\n"
+"\t/*\n"
+"\t  aa\n"
+"\t  b\n"
+"\t*/\n"
+"}",
+format("{\n"
+   "/*\n"
+   "  aa b\n"
+   "*/\n"
+   "}",
+   Tab));
   Tab.AlignConsecutiveAssignments = true;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -49,7 +49,7 @@
   /// this replacement. It is needed for determining how \p Spaces is turned
   /// into tabs and spaces for some format styles.
   void replaceWhitespace(FormatToken , unsigned Newlines, unsigned Spaces,
-