sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land.
In D57509#1378696 <https://reviews.llvm.org/D57509#1378696>, @kadircet wrote: > Dropping by: Maybe add `(fix available)` as a prefix rather than suffix. > Since long texts might end up getting truncated(especially in vim) Yeah truncation is definitely a problem, and on the other hand putting it at the start could get in the way and reads kind of backwards. Eric: up to you - if you move it to the front probably shorten to "[Fix]" or so. Another alternative is to use a prefix symbol like a bullet like we did with include insertion. As there, it's less explicit but also less intrusive. ================ Comment at: clangd/Diagnostics.cpp:164 /// note: candidate function not viable: requires 3 arguments -std::string mainMessage(const Diag &D) { +std::string mainMessage(const Diag &D, bool FixAvailable) { std::string Result; ---------------- I'd consider passing the number here, and distinguishing between "Fix" and "Fixes" in the message. Reasoning: when multiple fixes are available the user will have to make a choice. This may use a distinct UI (YCM) or not (VSCode), but foreshadowing the need for a choice will make the flow more predictable. I'd suggest *not* printing the actual number though, seems too noisy. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57509/new/ https://reviews.llvm.org/D57509 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
