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

Thu Jan 31 08:08:45 PST 2019

ioeric added a comment.

In D57509#1378707 <https://reviews.llvm.org/D57509#1378707>, @sammccall wrote:

> 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.

Yeah, this could be a concern for plugins that truncate messages (YCM+Vim). There are other editors that do not truncate e.g. vscode. So I'd suggest we start with something less intrusive like suffix and try it out. If it turned out to be a problem too often, we can explore options with prefix.

================
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;
----------------
sammccall wrote:
> 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.
Sounds good. Done.

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57509/new/

https://reviews.llvm.org/D57509