[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/FormattedString.cpp:110 +// Separate from next block. +*WritePtr++ = ' '; + } aganea wrote: > There's an issue at this line, the iterator

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

aganea added inline comments. Comment at: clang-tools-extra/clangd/FormattedString.cpp:110 +// Separate from next block. +*WritePtr++ = ' '; + } There's an issue at this line, the iterator might go past the end of the string (if running the

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

This revision was automatically updated to reflect the committed changes. Closed by commit rG597c6b65552a: [clangd] Introduce paragraph, the first part of new rendering structs (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 60763 tests passed, 0 failed and 726 were skipped. {icon check-circle color=green} clang-format: pass. Build artifacts : console-log.txt

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

kadircet updated this revision to Diff 233576. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71248/new/ https://reviews.llvm.org/D71248 Files:

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:79 + { + Test::PlainText, + "after", sammccall wrote: > I'd consider writing this as > `[&] { Para.appendText("after"); }` to make it

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/FormattedString.cpp:164 } - return R; + OS << '\n'; } this is worth a comment - we translate Paragraphs

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 60762 tests passed, 1 failed and 726 were skipped. failed: Clang.CodeGenCXX/runtime-dllstorage.cpp {icon check-circle color=green} clang-format: pass. Build artifacts

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

kadircet marked 12 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/FormattedString.cpp:122 +}; +std::string renderBlocks(llvm::ArrayRef> Children, + RenderType RT) { sammccall wrote: > I'm not

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

kadircet updated this revision to Diff 233566. kadircet marked 4 inline comments as done. kadircet added a comment. - Move separation logic from container to blocks - Add two concrete APIs to block for getting strings and update tests to use those. - Rename methods Repository: rG LLVM Github

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

sammccall added a comment. Design question around newlines :-) Otherwise looks good. Comment at: clang-tools-extra/clangd/FormattedString.cpp:95 +// Concatanates whitespace blocks into a single ` `. +std::string canonicalizeSpaces(std::string Input) { + // Goes over the

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 60657 tests passed, 0 failed and 726 were skipped. {icon check-circle color=green} clang-format: pass. Build artifacts : console-log.txt

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

kadircet updated this revision to Diff 233513. kadircet added a comment. - More tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71248/new/ https://reviews.llvm.org/D71248 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 60655 tests passed, 0 failed and 726 were skipped. {icon check-circle color=green} clang-format: pass. Build artifacts : console-log.txt

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

kadircet updated this revision to Diff 233420. kadircet marked 12 inline comments as done. kadircet added a comment. - Move code blocks out of Paragraph - Provide a way to control vertical spacing - Address comments around naming Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/FormattedString.cpp:95 +// Concatanates whitespace blocks into a single ` `. +std::string canonicalizeSpaces(std::string Input) { + // Goes over the string and preserves only a single ` ` for any whitespace

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

sammccall added a comment. Main thing here is I'm fairly sure that code blocks shouldn't be chunks in a paragraph (or I'm misunderstanding what a paragraph is) Comment at: clang-tools-extra/clangd/FormattedString.cpp:95 +// Concatanates whitespace blocks into a single ` `.

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

merge_guards_bot added a comment. Build result: pass - 60655 tests passed, 0 failed and 726 were skipped. Log files: console-log.txt , CMakeCache.txt

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

kadircet updated this revision to Diff 233078. kadircet added a comment. - More comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71248/new/ https://reviews.llvm.org/D71248 Files: clang-tools-extra/clangd/FormattedString.cpp

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

merge_guards_bot added a comment. Build result: pass - 60655 tests passed, 0 failed and 726 were skipped. Log files: console-log.txt , CMakeCache.txt

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

kadircet created this revision. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Initial patch for new rendering structs in clangd. Splitting implementation into smaller chunks, for a full view of the API see D71063