[PATCH] D44142: [clangd] Revamp handling of diagnostics.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added inline comments.


================
Comment at: clangd/ClangdLSPServer.cpp:329
+          {"title",
+           llvm::formatv("Apply FixIt {0}", GetFixitMessage(D.message))},
           {"command", ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND},
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > nit: while here, can we add a colon after FixIt (or if appropriate in practice, just drop the prefix altogether?). My recollection is this is a bit hard to parse.
> > I added a colon for now, but happy to look into removing the prefix. The use-case that's slightly confusing is:
> > 
> > ```use of undeclared identifier 'foo'. did you mean 'bar'?```
> > Since the actual fix-it is at the end, it's might be a bit hard to understand.
> Yeah. I'd still be inclined to drop the prefix:
>  - this case isn't ideal, but is also not terrible
>  - this is a "no separate note for fixit" case, where we can consider synthesizing the message from the text edit rather than reusing the diagnostic. That would work well at least in this case.
> 
I still find a prefix useful in some cases, e.g. the `unresolved identifier foo. did you mean bar?`.
Mostly for cases where a fix note uses the error message of the main diagnostic or when it's badly phrased (haven't really seen those, but I would be surprised if there are none).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44142