[PATCH] D64296: [clangd] Show documentation in hover, and fetch docs from index if needed.

[Sam McCall via Phabricator via llvm-commits]

sammccall marked 8 inline comments as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:148
+    bool IsBlock = C.Kind == ChunkKind::CodeBlock;
+    if (LastWasBlock.hasValue() && (IsBlock || *LastWasBlock))
+      if (!llvm::StringRef(C.Contents).endswith("\n\n"))
----------------
kadircet wrote:
> kadircet wrote:
> > could you add braces to outermost `if`, and clang-format this chunk.
> nit: if the following change(regarding cleaning whitespaces at the end of each chunk) was unintentional,
> why not make this one a lambda(like ensurewhitespaces) and just call it before and after we insert a `codeblock`s contents?
The problem then is if you insert two code blocks in a row, you get 4 newlines between them.


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:170
     }
-    llvm_unreachable("unhanlded ChunkKind");
+    while (!R.empty() && isWhitespace(R.back()))
+      R.pop_back();
----------------
kadircet wrote:
> why perform this for every chunk? This was previously used to get rid of trailing whitespaces, now this also trims trailing whitespaces in chunks. this doesn't sound valid to me?
This normalizes the whitespace between inline chunks to one space. Note that the text/inline blocks ensure there's a space before following another inline chunk. Is there a case where we'd be adding inline text and *want* multiple spaces between chunks?

This was an incomplete thought though: it allows the \n\n block to be simplified.

(I'm not totally happy with this rendering logic either, but I'm fairly convinced we're going to have to replace FormattedString with something hierarchical before too long, so I'm trying to avoid doing too much surgery here)


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1257
+
+  if (!Documentation.empty())
+  {
----------------
kadircet wrote:
> I believe documentation should come first. Since it has a non-fixed length, I believe it would be nice if people could simply not notice it(assuming bottom is closer to cursor location) WDYT?
The problem is that the hover text is often limited length and will almost certainly be truncated at the bottom. Hover also sometimes appears below the cursor.

Prioritizing from bottom->top is an interesting idea, but I don't think editors or users are expecting it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64296