Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support non project root language servers #11

Closed
wants to merge 1 commit into from
Closed

support non project root language servers #11

wants to merge 1 commit into from

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented Jul 22, 2020

@UziTech UziTech marked this pull request as draft July 22, 2020 21:19
@UziTech UziTech added the needs tests PR needs tests before merging label Dec 12, 2020
@aminya
Copy link
Member

aminya commented Jan 27, 2021

This branch is removed. Could you revive it?

@aminya
Copy link
Member

aminya commented May 5, 2021

@UziTech Could you rebase this? This is needed for flow-ide

https://github.com/steelbrain/flow-ide

Comment on lines -261 to -276
public determineProjectPath(textEditor: TextEditor): string | null {
const filePath = textEditor.getPath();
if (filePath == null) {
return null;
}
return this._normalizedProjectPaths.find((d) => filePath.startsWith(d)) || null;
}

public updateNormalizedProjectPaths(): void {
this._normalizedProjectPaths = atom.project.getDirectories().map((d) => this.normalizePath(d.getPath()));
}

public normalizePath(projectPath: string): string {
return !projectPath.endsWith(path.sep) ? path.join(projectPath, path.sep) : projectPath;
}

Copy link
Member

@aminya aminya May 5, 2021

Choose a reason for hiding this comment

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

I think we should just move this code to auto-languageclient (without modifying) so the user can override them. Maybe there is a better way to make this configurable instead of moving the methods

@@ -263,6 +274,7 @@ export default class AutoLanguageClient {
(filepath) => this.filterChangeWatchedFiles(filepath),
this.reportBusyWhile,
this.getServerName(),
(e) => this.determineProjectPath(e),
Copy link
Member

@aminya aminya May 5, 2021

Choose a reason for hiding this comment

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

Instead of all these, we can probably provide a method that is passed down to this._serverManager.

A property of AutoLanguageclient:

      public customDetermineProject: undefined | (textEditor: TextEditor) => string | null = undefined

If someone wants to use this, they should override it with a function. Later inside ServerManager.determineProject, we will check if this method is !== undefined, and if so we will use that instead.

@UziTech
Copy link
Member Author

UziTech commented May 5, 2021

I don't have this branch anymore. It would be easier for you to create a new PR that does this.

@UziTech UziTech closed this May 5, 2021
@aminya
Copy link
Member

aminya commented May 6, 2021

Yeah, I think it's easier to start over

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests PR needs tests before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants