[libcxx-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

[Christopher Di Bella via Phabricator via libcxx-commits]

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