subject:"\[PATCH\] D116859\: \[clang\-format\] Fix break being added to macro define with ColumnLimit\: 0"

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:19610
getLLVMStyleWithColumns(35));
-  verifyFormat("\"一 二 三 四 五 六 七 八 九 十\"",
-   getLLVMStyleWithColumns(31));
+  verifyFormat("\"一 二 三 四 五 六 七 八 九 十\"", getLLVMStyleWithColumns(31));
   verifyFormat("// Однажды в студёную зимнюю пору...",

Oops. Fixed post-commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:4810
+
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);

futuarmo wrote:
> owenpan wrote:
> > Please remove the newline and re-run `FormatTests`.
> FormatTests pass again
Done when landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Marek Kurdej via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG47a9eb2117aa: [clang-format] Fix break being added to macro 
define with ColumnLimit: 0 (authored by futuarmo, committed by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D116859?vs=399394=399906#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4804,6 +4804,13 @@
Style);
 }
 
+TEST_F(FormatTest, FormatsMacrosWithZeroColumnWidth) {
+  FormatStyle ZeroColumn = getLLVMStyleWithColumns(0);
+
+  verifyFormat("#define A LOOONG() LOOONG()",
+   ZeroColumn);
+}
+
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
   verifyFormat("#define A \\\n"
"  f({ \\\n"
@@ -19600,15 +19607,13 @@
 TEST_F(FormatTest, CountsUTF8CharactersProperly) {
   verifyFormat("\"ÐÐ´Ð½Ð°Ð¶Ð´Ñ Ð² ÑÑÑÐ´ÑÐ½ÑÑ Ð·Ð¸Ð¼Ð½ÑÑ Ð¿Ð¾ÑÑ...\"",
getLLVMStyleWithColumns(35));
-  verifyFormat("\"ä¸ äº ä¸ å äº å ä¸ å« ä¹ å\"",
-   getLLVMStyleWithColumns(31));
+  verifyFormat("\"ä¸ äº ä¸ å äº å ä¸ å« ä¹ å\"", getLLVMStyleWithColumns(31));
   verifyFormat("// ÐÐ´Ð½Ð°Ð¶Ð´Ñ Ð² ÑÑÑÐ´ÑÐ½ÑÑ Ð·Ð¸Ð¼Ð½ÑÑ Ð¿Ð¾ÑÑ...",
getLLVMStyleWithColumns(36));
   verifyFormat("// ä¸ äº ä¸ å äº å ä¸ å« ä¹ å", getLLVMStyleWithColumns(32));
   verifyFormat("/* ÐÐ´Ð½Ð°Ð¶Ð´Ñ Ð² ÑÑÑÐ´ÑÐ½ÑÑ Ð·Ð¸Ð¼Ð½ÑÑ Ð¿Ð¾ÑÑ... */",
getLLVMStyleWithColumns(39));
-  verifyFormat("/* ä¸ äº ä¸ å äº å ä¸ å« ä¹ å */",
-   getLLVMStyleWithColumns(35));
+  verifyFormat("/* ä¸ äº ä¸ å äº å ä¸ å« ä¹ å */", getLLVMStyleWithColumns(35));
 }
 
 TEST_F(FormatTest, SplitsUTF8Strings) {
@@ -19628,21 +19633,20 @@
 "\"Ð¿Ð¾ÑÑ,\"",
 format("\"ÐÐ´Ð½Ð°Ð¶Ð´Ñ, Ð² ÑÑÑÐ´ÑÐ½ÑÑ Ð·Ð¸Ð¼Ð½ÑÑ Ð¿Ð¾ÑÑ,\"",
getLLVMStyleWithColumns(13)));
+  EXPECT_EQ("\"ä¸ äº ä¸ \"\n"
+"\"å äºå \"\n"
+"\"ä¸ å« ä¹ \"\n"
+"\"å\"",
+format("\"ä¸ äº ä¸ å äºå ä¸ å« ä¹ å\"", getLLVMStyleWithColumns(11)));
   EXPECT_EQ(
-  "\"ä¸ äº ä¸ \"\n"
-  "\"å äºå \"\n"
-  "\"ä¸ å« ä¹ \"\n"
-  "\"å\"",
-  format("\"ä¸ äº ä¸ å äºå ä¸ å« ä¹ å\"", getLLVMStyleWithColumns(11)));
-  EXPECT_EQ("\"ä¸\t\"\n"
-"\"äº \t\"\n"
-"\"ä¸ å \"\n"
-"\"äº\t\"\n"
-"\"å \t\"\n"
-"\"ä¸ \"\n"
-"\"å«ä¹å\tqq\"",
-format("\"ä¸\täº \tä¸ å äº\tå \tä¸ å«ä¹å\tqq\"",
-   getLLVMStyleWithColumns(11)));
+  "\"ä¸\t\"\n"
+  "\"äº \t\"\n"
+  "\"ä¸ å \"\n"
+  "\"äº\t\"\n"
+  "\"å \t\"\n"
+  "\"ä¸ \"\n"
+  "\"å«ä¹å\tqq\"",
+  format("\"ä¸\täº \tä¸ å äº\tå \tä¸ å«ä¹å\tqq\"", getLLVMStyleWithColumns(11)));
 
   // UTF8 character in an escape sequence.
   EXPECT_EQ("\"aa\"\n"
@@ -19687,16 +19691,16 @@
 format("/* ÐÐ»ÑÐ¶Ñ, Ð¿Ð¾Ð´Ð½Ð¸Ð¼Ð°ÐµÑÑÑ Ð¼ÐµÐ´Ð»ÐµÐ½Ð½Ð¾ Ð² Ð³Ð¾ÑÑ\n"
" * ÐÐ¾ÑÐ°Ð´ÐºÐ°, Ð²ÐµÐ·ÑÑÐ°Ñ ÑÐ²Ð¾ÑÐ¾ÑÑÑ Ð²Ð¾Ð·. */",
getLLVMStyleWithColumns(13)));
-  EXPECT_EQ(
-  "/* ä¸äºä¸\n"
-  " * åäºåä¸\n"
-  " * å«  ä¹\n"
-  " * å  */",
-  format("/* ä¸äºä¸ åäºåä¸ å«  ä¹ å  */", getLLVMStyleWithColumns(9)));
+  EXPECT_EQ("/* ä¸äºä¸\n"
+" * åäºåä¸\n"
+" * å«  ä¹\n"
+" * å  */",
+format("/* ä¸äºä¸ åäºåä¸ å«  ä¹ å  */", getLLVMStyleWithColumns(9)));
   EXPECT_EQ("/* ð£ð®ð¼ð½ ð£ð¬ð²ð¯\n"
 " * ððªð¥ð\n"
 " * ðð¿ð±-ð  */",
-format("/* ð£ð®ð¼ð½ ð£ð¬ð²ð¯ ððªð¥ð ðð¿ð±-ð  */", getLLVMStyleWithColumns(12)));
+format("/* ð£ð®ð¼ð½ ð£ð¬ð²ð¯ ððªð¥ð ðð¿ð±-ð  */",
+   getLLVMStyleWithColumns(12)));
 }
 
 #endif // _MSC_VER
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -485,7 +485,8 @@
 // different LineFormatter would be used otherwise.
 if (Previous.ClosesTemplateDeclaration)
   return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
-if

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Armen Khachkinaev via Phabricator via cfe-commits

futuarmo added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:4810
+
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);

owenpan wrote:
> Please remove the newline and re-run `FormatTests`.
FormatTests pass again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Owen Pan via Phabricator via cfe-commits

owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:4810
+
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);

Please remove the newline and re-run `FormatTests`.



Comment at: clang/unittests/Format/FormatTest.cpp:4810-4811
+
+  verifyFormat("#define STRINGIFY(t) #t\n"
+   "#define MAKEVERSIONSTRING(x, y, z, build) STRINGIFY(x) \".\" 
STRINGIFY(y) \".\" STRINGIFY(z) \".\" STRINGIFY(build)\n",
+   ZeroColumn);

curdeius wrote:
> HazardyKnusperkeks wrote:
> > curdeius wrote:
> > > Please test with something simpler.
> > Did this show the bugged behavior without the patch?
> > 
> > Regarding the `// clang-format off` there is the question what do we want 
> > in the tests? Show the long lines as the long lines, then we need to turn 
> > it off some times. Or do we want to keep the limit and break the strings, 
> > thus making it harder to read for the human?
> > Did this show the bugged behavior without the patch?
> Yes. I checked with a pretty fresh master.
> 
> > Regarding the `// clang-format off` there is the question what do we want 
> > in the tests? Show the long lines as the long lines, then we need to turn 
> > it off some times. Or do we want to keep the limit and break the strings, 
> > thus making it harder to read for the human?
> I'm pretty much against adding `// clang-format off` too, it hurts more than 
> it helps IMO.
> Regarding the `// clang-format off` there is the question what do we want in 
> the tests? Show the long lines as the long lines, then we need to turn it off 
> some times. Or do we want to keep the limit and break the strings, thus 
> making it harder to read for the human?

I prefer the latter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Armen Khachkinaev via Phabricator via cfe-commits

futuarmo added a comment.

@curdeius, all tests pass on fresh build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread MyDeveloperDay via Phabricator via cfe-commits

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

Assuming all other tests pass, I'm ok


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added a comment.

@MyDeveloperDay, do you have any more objections?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-12 Thread Armen Khachkinaev via Phabricator via cfe-commits

futuarmo added a comment.

I will build fresh main branch tomorrow (it tooks 1+ hour) and will comment 
here about test results
I don't have commit rights, so it will be great if someone commit it with user 
"Armen Khachkinaev" and email "armen...@yandex.ru"
Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-12 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius accepted this revision.
curdeius added a comment.

LGTM. Thanks for working on this!
But before landing, please retest the original misformatted code from the bug 
report with a freshly built clang-format.

Also, do you have commit rights? If not, do you want someone to land it on your 
behalf? If so, please send your name and email address to be used for the 
contribution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

10 matches

Site Navigation

Mail list logo

Footer information