[PATCH] D57509: [clangd] Append "(fix available)" to diagnostic message when fixes are present.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 31 07:44:28 PST 2019


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





More information about the cfe-commits mailing list