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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to