[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 6 10:14:06 PDT 2019


ilya-biryukov added inline comments.


================
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;;) {
----------------
sammccall wrote:
> ```
> backticks = "```"
> while (Input.contains(backticks))
>   backticks.push_back("`")
> ```
> 
> I don't think we care about DOS here?
I'd like to keep this linear anyway.
Simplified the algorithm a bit, let me know if it still looks too complicated.


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:107
+void FormattedString::appendInlineCode(std::string Code) {
+  assert(!llvm::StringRef(Code).contains("\n"));
+
----------------
sammccall wrote:
> 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)
Actually, the [[ https://spec.commonmark.org/0.29/#code-spans | CommonMark specification ]] allows newline in the inline code blocks.
The renderers should replace newlines with spaces.
Removed the assertion.


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:140
+
+static bool IsWhitespace(char C) {
+  return llvm::StringLiteral(" \t\n").contains(C);
----------------
sammccall wrote:
> (we could also use the version in CharInfo)
Thanks! I knew there should be a helper like this somewhere in LLVM...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547





More information about the cfe-commits mailing list