[PATCH] D71555: [clangd] Refurbish HoverInfo::present

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:466
+  markup::Paragraph &P = Output.addParagraph();
+  P.appendText(beautify(index::getSymbolKindString(Kind)));
+  if (!Name.empty()) {
----------------
sammccall wrote:
> kadircet wrote:
> > sammccall wrote:
> > > is beautify() actually better here? I actually quite like the lowercase as it reduces the emphasis on the kind vs the name, and the hyphens as it makes it easier to pick out the name.
> > > 
> > > This is up to you of course.
> > I've actually found those hyphens annoying, but don't have any strong feelings towards those. I would land it this way and see how ppl feel about it. (but of course, if you've got any strong feelings we could also do it the other way around)
> ok, that's fine. If you don't care much about the case, consider dropping the case transform and just replacing `-` with ` `, it seems less noisy. But up to you, fine to leave as-is.
got rid of beautify, printing getSymbolKindString as is.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:495
+    markup::Paragraph &P = Output.addParagraph();
+    P.appendText("Value: ");
+    P.appendCode(*Value);
----------------
sammccall wrote:
> consider "Value = " or even just "= "
sorry somehow missed that one.

what about putting that into header part with something like:

```
variable `var` : `int`(=3)

function `foo` -> `int`(=3)
```

we can also drop equals signs, I am not sure if it looks confusing without those.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:490
+  markup::Paragraph &Header = Output.addParagraph();
+  Header.appendText(beautify(index::getSymbolKindString(Kind)));
+  if (!Name.empty()) {
----------------
sammccall wrote:
> if there's no name, do we want to skip the header entirely?
> (I can't think of examples for this case.)
`HoverInfo::Name` is always filled currently, as we always get a `NamedDecl`(after change to `explicitReferenceTargets`).

Changing it accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71555