mprobst marked 2 inline comments as done. mprobst added inline comments.
================ Comment at: clang/lib/Format/Format.cpp:2531 + }); + auto Env = ---------------- MyDeveloperDay wrote: > Ok, this comment is more a discussion point rather than a review comment. Was > there a reason you needed to put the TrailingCommaInserter pass after the > Formatter pass? (maybe you needed the Tokens annotated?) > > From my understanding of some of the other conversations, it seemed the > reason for ignoring the Column limit was because the "," insertion came after > it had been formatted (is that correct?) > > If there was a good reason that's also fine, I was just interested to learn > if there was some part of the Formatter that perhaps needs to be pulled out > into its own separate pass. > > The problem with inserting the comma during formatting is that it'd need to be inserted during the state exploration as part of Dijkstra's implemented in clang-format. I've considered that, but it'd be complex (if we make formatting decision X, we now add a token, which might invalidate the formatting decision). Keeping it as a separate pass has drawbacks, such as potentially not ending up with perfect formatting, thus the backing off to insert over ColumnLimit, but seems overall simpler. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73354/new/ https://reviews.llvm.org/D73354 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
