This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Add support for vscode extension configuration
ClosedPublic

Authored by stanionascu on Mar 19 2017, 3:09 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

stanionascu created this revision.Mar 19 2017, 3:09 AM
stanionascu added a reviewer: cfe-commits.
krasimir added inline comments.Mar 20 2017, 3:46 AM
clangd/clients/clangd-vscode/package.json
45 ↗(On Diff #92268)

Maybe prepend a , for example: /usr/bin/clangd to the "description".

clangd/clients/clangd-vscode/src/extension.ts
19 ↗(On Diff #92268)

I feel that this is too platform-specific and redundant because "clangd.path" already has a default value.

krasimir edited edge metadata.Mar 20 2017, 5:29 AM

It looks pretty good! I just have a minor suggestion.

clangd/clients/clangd-vscode/package.json
45 ↗(On Diff #92268)

meh, it's OK like that.

stanionascu edited the summary of this revision. (Show Details)
  • removed unrelated new line change
  • extended the "clang.path" description a bit
  • removed defaultValue for getConfig<>, as it's anyway "contributed" by extension package.json
stanionascu marked 2 inline comments as done.Mar 20 2017, 11:48 AM
stanionascu added inline comments.
clangd/clients/clangd-vscode/package.json
45 ↗(On Diff #92268)

I still extended the description with the example :)

krasimir accepted this revision.Mar 23 2017, 8:20 AM

Looks good! Sorry for the delay.

This revision is now accepted and ready to land.Mar 23 2017, 8:20 AM

Thanks for review!
Would be great if somebody would commit it, as I cannot do it myself.

This revision was automatically updated to reflect the committed changes.