This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Introduce codeblocks
ClosedPublic

Authored by kadircet on Dec 12 2019, 5:45 AM.

Details

Summary

Follow-up to the patch D71248

Diff Detail

Event Timeline

kadircet created this revision.Dec 12 2019, 5:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 12 2019, 5:45 AM
sammccall added inline comments.Dec 12 2019, 6:02 AM
clang-tools-extra/clangd/FormattedString.cpp
138

can you inline renderCodeBlock here, or modify it to just compute the marker?
It doesn't make sense to have this function just to delegate to one with a different interface (and a copy)

142

This means if we have two consecutive code blocks, they'll be separated by two blank lines...

worth at least a fixme.

(I guess the "right thing" here is to have virtual unsigned verticalMargin() { return 0; }, and have the document renderer insert max(first.verticalMargin(),second.verticalMargin()) newlines in between). Probably not worth it

sammccall accepted this revision.Dec 12 2019, 6:02 AM

Otherwise LG

This revision is now accepted and ready to land.Dec 12 2019, 6:02 AM
kadircet updated this revision to Diff 233606.Dec 12 2019, 6:26 AM
kadircet marked 4 inline comments as done.
  • Address comments
clang-tools-extra/clangd/FormattedString.cpp
138

changed it to only return Marker

142

adding a testcase and a fixme

sammccall accepted this revision.Dec 12 2019, 6:27 AM

Unit tests: pass. 60768 tests passed, 0 failed and 726 were skipped.

clang-format: pass.

Build artifacts: console-log.txt, CMakeCache.txt, test-results.xml, diff.json

This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Dec 14 2019, 7:15 AM
thakis added inline comments.
clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
126

Older (but still supported) gccs can't handle multiline raw strings in macro arguments, see e.g. http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21230/steps/ninja%20check%201/logs/stdio

I fixed this for you in 687e98d294c4f77e. It's been broken for 3 days, please watch bots and your inbox after committing.