Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

Add missing project method for plugins #462

Merged
merged 4 commits into from
May 30, 2018
Merged

Add missing project method for plugins #462

merged 4 commits into from
May 30, 2018

Conversation

soywod
Copy link
Contributor

@soywod soywod commented May 18, 2018

I'm using tslint-language-service, and this plugin doesn't work with javascript-typescript-langserver because there is some missing properties/methods in your PluginCreateInfo. So I just added the getCurrentDirectory.

in fact, many other methods have to be set there, since Project
implements LanguageServiceHost.
@soywod soywod changed the title Add missing project method Add missing project method for plugins May 18, 2018
@codecov
Copy link

codecov bot commented May 18, 2018

Codecov Report

Merging #462 into master will decrease coverage by 0.04%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #462      +/-   ##
==========================================
- Coverage    83.1%   83.06%   -0.05%     
==========================================
  Files          15       15              
  Lines        2042     2043       +1     
  Branches      416      483      +67     
==========================================
  Hits         1697     1697              
- Misses        343      344       +1     
  Partials        2        2
Impacted Files Coverage Δ
src/plugins.ts 84.84% <ø> (ø) ⬆️
src/project-manager.ts 87.07% <0%> (-0.24%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 242e5a0...e1c1437. Read the comment docs.

src/plugins.ts Outdated
@@ -40,6 +40,7 @@ export interface PluginCreateInfo {
* The portion of tsserver's Project API exposed to plugins
*/
export interface Project {
getCurrentDirectory: () => string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit, could this just be a method rather than an arrow function?

getCurrentDirectory(): string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you right, I update this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

project: { projectService: { logger: this.logger } }, // TODO: may need more support
project: {
// TODO: may need more support
getCurrentDirectory: this.getHost().getCurrentDirectory.bind(this),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bind() is not type safe, prefer wrapping in a function manually

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

@felixfbecker felixfbecker merged commit faa787c into sourcegraph:master May 30, 2018
@felixfbecker
Copy link
Contributor

🎉 This PR is included in version 2.9.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants