[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