kadircet marked an inline comment as done. kadircet added a comment. thanks, mostly LG apart from dropping of white-space only comments.
================ Comment at: clang-tools-extra/clangd/CodeComplete.cpp:377 + auto SetDoc = [&](llvm::StringRef Doc) { + if (!Doc.trim().empty()) { + Completion.Documentation.emplace(); ---------------- why do we ignore whitespace only comments now ? nit: early exit; ``` if(Doc.trim().empty()) return; ... ``` ================ Comment at: clang-tools-extra/clangd/Hover.h:83 // Try to infer structure of a documentation comment (e.g. line breaks). +// FIXME: move to another file so CodeComplete doesn't depend on Hover. void parseDocumentation(llvm::StringRef Input, markup::Document &Output); ---------------- yeah was about to say that :D what about formattedstring.h with a different name like `parseRaw` ? ================ Comment at: clang-tools-extra/clangd/Protocol.cpp:317 + for (const auto &Format : *DocumentationFormat) { + if (fromJSON(Format, K)) { + R.CompletionDocumentationFormat = K; ---------------- why not just `if (fromJson(Format, R.CompletionDocFormat)` ? ================ Comment at: clang-tools-extra/clangd/Protocol.cpp:347 for (const auto &Format : *ContentFormat) { - MarkupKind K = MarkupKind::PlainText; if (fromJSON(Format, K)) { R.HoverContentFormat = K; ---------------- i know it is not related to this change, but you've touched it :P so same as above, maybe just `fromJson(Format, R.HoverContentFormat)` ================ Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:835 /* Multi-line block comment */ ---------------- no action needed here(apart from maybe filing a bug), but this one looks like a hard-break to me, as the line below is longer in width. why is parseDocumentation not preserving it? It might also be the case that i am just wrong, as I didn't check the line break heuristics :D 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