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

2019-01-31 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. > I don't think it's pure noise. Vscode displays diagnostics in the "PROBLEMS" > tab. A suffix allows you to tell whether fixes are available without hovering > on the errors. And it shows bulb icons when you hover over diagnostics in this dialog too. To be

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

2019-01-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a subscriber: hokein. ioeric added a comment. In D57509#1378978 , @ilya-biryukov wrote: > In D57509#1378943 , @ioeric wrote: > > > Yes, but a new option seems a bit of an overkill here. The text is

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

2019-01-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. FWIW, I don't think this is worth adding an option or protocol extensions for at this point. There are pros and cons to showing this in VSCode (pros are visibility without hovering, particularly in problems view and cross-editor consistency, cons are certainly noise

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

2019-01-31 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In D57509#1378943 , @ioeric wrote: > Yes, but a new option seems a bit of an overkill here. The text is appended > and doesn't seem to affect the readability of original diagnostic message > much (IMO). Could you

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

2019-01-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In D57509#1378928 , @ilya-biryukov wrote: > Could this be made optional for VSCode? > As mentioned in the discussion before, I would personally prefer to turn it > off, even though I do acknowledge the need for this for some

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

2019-01-31 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Could this be made optional for VSCode? As mentioned in the discussion before, I would personally prefer to turn it off, even though I do acknowledge the need for this for some clients, e.g. vim. Repository: rL LLVM CHANGES SINCE LAST ACTION

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

2019-01-31 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL352764: [clangd] Append (fix available) to diagnostic message when fixes are present. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE

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

2019-01-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In D57509#1378707 , @sammccall wrote: > In D57509#1378696 , @kadircet wrote: > > > Dropping by: Maybe add `(fix available)` as a prefix rather than suffix. > > Since long texts might end up

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

2019-01-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 184510. ioeric marked 2 inline comments as done. ioeric added a comment. - address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57509/new/ https://reviews.llvm.org/D57509 Files:

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

2019-01-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. In D57509#1378696 , @kadircet wrote: > Dropping by: Maybe add `(fix available)` as a prefix rather than suffix. > Since long texts might end up

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

2019-01-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Dropping by: Maybe add `(fix available)` as a prefix rather than suffix. Since long texts might end up getting truncated(especially in vim) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57509/new/

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

2019-01-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. This would make diagnostic fixits more discoverable, especially for plugins like YCM. Repository: rCTE Clang Tools Extra