erichkeane added a comment. In D138939#3964439 <https://reviews.llvm.org/D138939#3964439>, @erichkeane wrote:
> In D138939#3964404 <https://reviews.llvm.org/D138939#3964404>, @cjdb wrote: > >> 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? > > Ah, oh... is the Sarif formatting being done with a new formatter? That > seems unfortunate, since folks using the other formatters won't be able to > use the user friendly formats. I've been alerted offline that I am misunderstanding the Sarif proposal, and where this is going. I'll note that I wasn't present/invited at the calls where all of this was discussed, so I am admittedly not completely up to date. The above concern shouldn't stop others from reviewing this, particularly if you better understand the intent here. 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
