krasimir added inline comments.
================ Comment at: include/clang/Format/Format.h:1375 + std::vector<std::string> EnclosingFunctionNames; + /// \brief The canonical delimiter for this language. + std::string CanonicalDelimiter; ---------------- djasper wrote: > Can you pull apart this patch? > > In my view, it has three parts that have an ordering, but are actually fairly > independent: > > 1. Propagate all configured languages to the formatting library. First patch > to land, should not affect the visible behavior. > 2. Restructure RawStringFormat to be centered around each language. This is a > restructuring to make things easier and use #1. > 3. Add a CanonicalDelimiter and make clang-format canonicalize it. > > I'll focus my comments on what's required for #1 for now as that is already > complicated (IMO). I believe these should all go together: the reason that we propagate all configured languages to the formatting library is to be able to use them as a replacement for the BasedOnStyle in RawStringFormat. To make this possible, we need to update the internal structure of RawStringFormat itself to base it around each language. The canonical delimiter part is just a convenience for this I guess, which could be split. My biggest concern with (1) is that since it has no visible behavior and no other uses except for the adaptation of (2), it is not testable by itself and it's not evident that a patch doing just (1) would handle the things correctly. Repository: rC Clang https://reviews.llvm.org/D40909 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
