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

[Sam McCall via Phabricator via llvm-commits]

sammccall marked an inline comment as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:170
     }
-    llvm_unreachable("unhanlded ChunkKind");
+    while (!R.empty() && isWhitespace(R.back()))
+      R.pop_back();
----------------
kadircet wrote:
> sammccall wrote:
> > 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)
> > 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?
> 
> I am not sure if we would ever want to put some spaces between chunks, but it is still doable by prepending it to the second chunk instead of appending to the first.
> 
> So, I suppose it is OK until we figure out some other use case.
Yeah, I think that's unlikely to do by accident. Mostly I don't want to worry about whether a doc comment or prettyprinted decl or whatever ends in \n.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64296