sammccall added a comment.
As discussed, it probably makes sense to split into two patches, adding this
abstraction and using it.
(All together is also fine but should have the tests for the new behavior,
which probably means renderForTests() too)
Generally looks good. I think we'll run into the limitations as soon as we want
to add lists, but that's OK.
================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:71
+ // start and end the code block with more.
+ unsigned MaxBackticks = 0;
+ for (llvm::StringRef Left = Input;;) {
----------------
```
backticks = "```"
while (Input.contains(backticks))
backticks.push_back("`")
```
I don't think we care about DOS here?
================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:90
+void FormattedString::appendText(std::string Text) {
+ if (Chunks.empty() || Chunks.back().Kind != ChunkKind::PlainText) {
+ Chunk C;
----------------
comment for this merge?
================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:107
+void FormattedString::appendInlineCode(std::string Code) {
+ assert(!llvm::StringRef(Code).contains("\n"));
+
----------------
I'm not sure we're realistically going to be able to enforce this very well,
we're going to use this for types. Maybe translate into ' ' instead? (or at
least in production mode)
================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:140
+
+static bool IsWhitespace(char C) {
+ return llvm::StringLiteral(" \t\n").contains(C);
----------------
(we could also use the version in CharInfo)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58547/new/
https://reviews.llvm.org/D58547
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
