sammccall marked an inline comment as done.
sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:377
+ auto SetDoc = [&](llvm::StringRef Doc) {
+ if (!Doc.trim().empty()) {
+ Completion.Documentation.emplace();
----------------
kadircet wrote:
> sammccall wrote:
> > kadircet wrote:
> > > why do we ignore whitespace only comments now ?
> > >
> > > nit: early exit;
> > > ```
> > > if(Doc.trim().empty())
> > > return;
> > > ...
> > > ```
> > > why do we ignore whitespace only comments now ?
> >
> > Having the empty-check at all is because the type changed from
> > string-or-empty to document-or-null.
> >
> > I threw the trim in "while there" because ws-only seems
> > "functionally-empty" - happy to revert on the basis of "unrelated change"
> > but is there an actual reason to include these? Or just the wrong layer?
> the trimming behaviour is already performed by FormattedString in appendText,
> as we cannonicalizeSpaces and ignore the chunk if it is whitespace only
> during the process. so i was confused to see it here too.
>
> Previous code would accept and surface white-space only comment as-is. Now
> we'll ignore such comments (other options is displaying an empty comment).
>
> I suppose doing the check below in LSP conversion might be more clear, since
> from code completion's perspective we've actually found a comment unless it
> is empty.
> the trimming behaviour is already performed by FormattedString in appendText,
> as we cannonicalizeSpaces and ignore the chunk if it is whitespace only
> during the process. so i was confused to see it here too.
Yeah, this is too late - we want to avoid creating the markup::Document at all.
Nevertheless, reverted the change, it's basically unrelated and I don't have a
test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79157/new/
https://reviews.llvm.org/D79157
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
