sstwcw added inline comments.

================
Comment at: clang/include/clang/Format/Format.h:157
+  ///   a   &= 2;
+  ///   bbb  = 2;
+  ///
----------------
HazardyKnusperkeks wrote:
> sstwcw wrote:
> > HazardyKnusperkeks wrote:
> > > sstwcw wrote:
> > > > HazardyKnusperkeks wrote:
> > > > > curdeius wrote:
> > > > > > I guess it would be complicated to avoid adding an additional space 
> > > > > > here. I mean, it could be:
> > > > > > ```
> > > > > > a  &= 2;
> > > > > > bbb = 2;
> > > > > > ```
> > > > > > And with 3-char operators, there's one more space.
> > > > > That would be awesome, but it should be an option to turn off or on.
> > > > > But I think this would really be complicated.
> > > > I can do it either way. But I thought without the extra space the 
> > > > formatted code looked ugly, especially when mixing `>>=` and `=`.  
> > > > Which way do you prefer?
> > > > 
> > > > 
> > > > ```
> > > > a >>= 2;
> > > > bbb = 2;
> > > > ```
> > > I would prefer an option, to be able to do both.
> > > I think I would use the variant which is more compact, but can't say for 
> > > sure until I really have the option and tried it.
> > You mean like a new entry under `AlignConsecutiveStyle` with the options to 
> > align the left ends of the operators and to align the equal sign with and 
> > without padding them to the same length? I don't want the new option to be 
> > merged with `AlignCompound`, because we are working on adding support for a 
> > language that uses both `=` and `<=` for regular assignments, and maybe 
> > someone will want to support a language that has both `=` and `:=` in the 
> > future.
> Yeah. Basing on @MyDeveloperDay 's suggestion:
> ```
> struct AlignConsecutiveStyle {
>   bool Enabled;
>   bool AcrossComments;
>   bool AcrossEmptyLines;
>   bool AlignCompound;
>   bool CompactWhitespace; // This is just a suggestion, I'm open for better 
> names.
> };
> ```
I added the new option. Moving the old options into the struct turned out to be 
too much work for me on a weekend. If you can accept the configuration style in 
the current revision, I will probably move the old options into the struct at 
some later date. By the way, what is the purpose of `FormatStyle::operator==`? 
It looks like it does not compare`BraceWrapping`.


================
Comment at: clang/unittests/Format/FormatTest.cpp:16852
-  // otherwise the function parameters will be re-flowed onto a single line.
-  Alignment.ColumnLimit = 0;
   EXPECT_EQ("int    a(int   x,\n"
----------------
sstwcw wrote:
> MyDeveloperDay wrote:
> > Huge `no` from me, don't change the tests because they break based on your 
> > change.
> Here is the problem in isolation: https://reviews.llvm.org/D119625.  What do 
> you suggest about this test?
I hadn't noticed that column limit = 0 meant no limit. This test doesn't really 
need to change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

Reply via email to