[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

[Paweł Żukowski via cfe-commits]

idlecode added a comment.

Thanks for pointing it out, just a minute ago I found a proper document <http://llvm.org/docs/Phabricator.html> mentioning it (I have no idea how I could miss it).
I hope to be more use in future :)



================
Comment at: unittests/Format/FormatTest.cpp:11365
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  std::string left = "int a = 1 << 2; /* foo\n"
+                     "                   bar */";
----------------
djasper wrote:
> It's always useful to have some other formatting being done in the same test. We repeatedly ran into cases in the past where a test only passed because some change effectively disabled formatting for a specific line. I suggest writing these as:
> 
>   EXPECT_EQ("int a = 1 << 2; /* foo\n"
>             "                   bar */",
>             format("int    a=1<<2;  /* foo\n"
>                    "                   bar */"));
Oh, that is worth mentioning, thanks :)


================
Comment at: unittests/Format/FormatTest.cpp:11365
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  std::string left = "int a = 1 << 2; /* foo\n"
+                     "                   bar */";
----------------
idlecode wrote:
> djasper wrote:
> > It's always useful to have some other formatting being done in the same test. We repeatedly ran into cases in the past where a test only passed because some change effectively disabled formatting for a specific line. I suggest writing these as:
> > 
> >   EXPECT_EQ("int a = 1 << 2; /* foo\n"
> >             "                   bar */",
> >             format("int    a=1<<2;  /* foo\n"
> >                    "                   bar */"));
> Oh, that is worth mentioning, thanks :)
Oh, that is good to know; Done


https://reviews.llvm.org/D25439