cjdb marked 3 inline comments as done. cjdb added a comment. In D138939#3963496 <https://reviews.llvm.org/D138939#3963496>, @erichkeane wrote:
> In D138939#3963473 <https://reviews.llvm.org/D138939#3963473>, @tschuett > wrote: > >> Maybe `void FormatDiagnostic(SmallVectorImpl<char> &OutStr, DiagnosticMode >> mode)`instead of `void FormatDiagnostic(SmallVectorImpl<char> &OutStr)`? >> To make the transition easer and future proof. > > I like this idea. Perhaps with DiagnosticMode being a 3-way enum: > > enum struct DiagnosticMode { > Legacy, > Sarif, > Default = Legacy > } > > I like the idea in particular, since it makes a command line flag to modify > "Default" to be whichever the user prefers pretty trivial. There's already a flag for this: `-fdiagnostics-format=sarif`. Why do we need a second diagnostic mode flag? ================ Comment at: clang/tools/clang-format/ClangFormat.cpp:397 + Info.FormatSummary(vec); + Info.FormatLegacyReason(vec); errs() << "clang-format error:" << vec << "\n"; ---------------- rymiel wrote: > I don't think this indent change was intended? I find it ironic that clang-format misformatted ClangFormat.cpp. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138939/new/ https://reviews.llvm.org/D138939 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
