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