This is an archive of the discontinued LLVM Phabricator instance.

Differential D65373

[clangd] Update features table in the docs with links to LSP extension proposals
ClosedPublic

Authored by nridge on Jul 27 2019, 6:52 PM.

Details

Reviewers
sammccall
Commits
rCTE369229: [clangd] Update features table in the docs with links to LSP extension proposals
rGdfe5f3eb0a00: [clangd] Update features table in the docs with links to LSP extension proposals
rL369229: [clangd] Update features table in the docs with links to LSP extension proposals
Summary

Also update the semantic coloring entry to reflect it being supported in clangd now.

Diff Detail

Event Timeline

nridge created this revision.Jul 27 2019, 6:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2019, 6:52 PM
Herald added subscribers: cfe-commits, kadircet, arphaman and 3 others. · View Herald Transcript
Harbormaster completed remote builds in B35729: Diff 212075.Jul 27 2019, 6:53 PM
nridge marked an inline comment as done.Jul 27 2019, 6:55 PM
nridge added inline comments.
clang-tools-extra/docs/clangd/Features.rst
266

Note, I changed "Organize Includes" to "Yes" because it doesn't need a new protocol: it's meant to use textDocument/codeAction (there's even a CodeActionKind for it, source.organizeImports).

nridge added a comment.Aug 11 2019, 4:29 PM

Review ping :)

ilya-biryukov added inline comments.Aug 12 2019, 1:23 AM
clang-tools-extra/docs/clangd/Features.rst
260

Could you mention that semantic coloring is implemented in clangd? (with a footnote that it only works in Theia atm)

269

Having the same marker for three links makes is hard to read. WDYT about using the footnote syntax here?

Proposed [1]_
Proposed [2]_


.. [1] https://github.com/microsoft/language-server-protocol/issues/468
.. [2] https://github.com/microsoft/language-server-protocol/issues/136

Just a suggestion, not sure which of the two versions renders nicer.

sammccall accepted this revision.Aug 12 2019, 5:38 AM

Sorry, I had marked this as "accepted", but not hit send. I agree with Ilya's comments, but please go ahead and land this when ready.

(I wish Phabricator didn't say "sammccall accepted this revision" when it's not true - I even went back to check that I'd accepted it)

This revision is now accepted and ready to land.Aug 12 2019, 5:38 AM
nridge updated this revision to Diff 215815.Aug 18 2019, 10:04 PM

Address review comments

Harbormaster completed remote builds in B36927: Diff 215815.Aug 18 2019, 10:05 PM
nridge marked 2 inline comments as done.Aug 18 2019, 10:09 PM
nridge added inline comments.
clang-tools-extra/docs/clangd/Features.rst
260

Updated. I didn't add a footnote, because it doesn't seem any different than e.g. type hierarchy which I think is also only supported by Theia at the moment -- clangd implements the extension as proposed, and it will work with any client that does as well.

nridge edited the summary of this revision. (Show Details)Aug 18 2019, 10:09 PM
Closed by commit rL369229: [clangd] Update features table in the docs with links to LSP extension proposals (authored by nridge). · Explain WhyAug 18 2019, 10:10 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 18 2019, 10:10 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript

Revision Contents

PathSize
clang-tools-extra/
docs/
clangd/
12 lines

Diff 212075

clang-tools-extra/docs/clangd/Features.rst

Show First 20 Lines • Show All 251 Lines • ▼ Show 20 Lines
| Quick Assist | Yes | No | | Quick Assist | Yes | No |
+-------------------------------------+------------+----------+ +-------------------------------------+------------+----------+
| Hide Method | Yes | No | | Hide Method | Yes | No |
+-------------------------------------+------------+----------+ +-------------------------------------+------------+----------+
| Implement Method | Yes | No | | Implement Method | Yes | No |
+-------------------------------------+------------+----------+ +-------------------------------------+------------+----------+
| Gen. Getters/Setters | Yes | No | | Gen. Getters/Setters | Yes | No |
+-------------------------------------+------------+----------+ +-------------------------------------+------------+----------+
| Syntax and Semantic Coloring | No | No | | Syntax and Semantic Coloring |`Proposed`__| No |
ilya-biryukovUnsubmitted
Not Done
ReplyInline Actions

Could you mention that semantic coloring is implemented in clangd? (with a footnote that it only works in Theia atm)

ilya-biryukov: Could you mention that semantic coloring is implemented in clangd? (with a footnote that it…
nridgeAuthorUnsubmitted
Done
ReplyInline Actions

Updated. I didn't add a footnote, because it doesn't seem any different than e.g. type hierarchy which I think is also only supported by Theia at the moment -- clangd implements the extension as proposed, and it will work with any client that does as well.

nridge: Updated. I didn't add a footnote, because it doesn't seem any different than e.g. type…
+-------------------------------------+------------+----------+ +-------------------------------------+------------+----------+
| Call hierarchy | No | No | | Call hierarchy |`Proposed`__| No |
+-------------------------------------+------------+----------+ +-------------------------------------+------------+----------+
| Type hierarchy | No | Yes | | Type hierarchy |`Proposed`__| Yes |
+-------------------------------------+------------+----------+ +-------------------------------------+------------+----------+
| Organize Includes | No | No | | Organize Includes | Yes | No |
nridgeAuthorUnsubmitted
Done
ReplyInline Actions

Note, I changed "Organize Includes" to "Yes" because it doesn't need a new protocol: it's meant to use textDocument/codeAction (there's even a CodeActionKind for it, source.organizeImports).

nridge: Note, I changed "Organize Includes" to "Yes" because it doesn't need a new protocol: it's meant…
+-------------------------------------+------------+----------+ +-------------------------------------+------------+----------+
__ https://github.com/microsoft/language-server-protocol/issues/18
ilya-biryukovUnsubmitted
Done
ReplyInline Actions

Having the same marker for three links makes is hard to read. WDYT about using the footnote syntax here?

Proposed [1]_
Proposed [2]_


.. [1] https://github.com/microsoft/language-server-protocol/issues/468
.. [2] https://github.com/microsoft/language-server-protocol/issues/136

Just a suggestion, not sure which of the two versions renders nicer.

ilya-biryukov: Having the same marker for three links makes is hard to read. WDYT about using the footnote…
__ https://github.com/microsoft/language-server-protocol/issues/468
__ https://github.com/microsoft/language-server-protocol/issues/136