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

get_project_path() incorrectly determines path #79

Closed
rmccue opened this issue Sep 7, 2017 · 5 comments · Fixed by #81
Closed

get_project_path() incorrectly determines path #79

rmccue opened this issue Sep 7, 2017 · 5 comments · Fixed by #81

Comments

@rmccue
Copy link
Contributor

rmccue commented Sep 7, 2017

If you have multiple sibling directories in a workspace with common prefix characters, it will incorrectly use a partial path for this.

I have two directories open, which are ~/Projects/example1 and ~/Projects/example2. os.path.commonprefix() will hence return ~/Projects/example, which is not a directory that exists. (The Python docs note that os.path.commonpath() should be used instead.)

This approach is generally problematic anyway. I have two subdirectories deep inside a common directory, and even without the partial filename problem, this is still problematic for me as it causes a lot more files than I care about to be indexed. This causes the initial parsing to take roughly forever.

VS Code is currently working on adding multi-root support (microsoft/vscode#28344), so I suspect they'll add support to the Language Server protocol at some point. The current behaviour in the beta is just to index the first directory in the project. IMO, this is the best behaviour until the protocol actually supports multi-root.

@rmccue
Copy link
Contributor Author

rmccue commented Sep 7, 2017

I suspect they'll add support to the Language Server protocol at some point

It appears they've got early work underway on this: microsoft/vscode-languageserver-node@c3a880f

@tomv564
Copy link
Contributor

tomv564 commented Sep 7, 2017

Thanks for the detailed bug report, sounds like commonpath is the intended behaviour.

I am myself not convinced of LSP's shared-root-of-multiple-folders approach, but would only supporting the first folder be an improvement?

@rmccue
Copy link
Contributor Author

rmccue commented Sep 8, 2017

would only supporting the first folder be an improvement?

I definitely think so; the way I use Sublime projects typically is by having a main directory with multiple auxiliary directories for easy access (for example, a component of a large system along with the core library for said system). Having all of it integrated would be nice, but in lieu of that, just using the first folder I think is the best behaviour (and matches VS Code's current behaviour).

@rmccue
Copy link
Contributor Author

rmccue commented Sep 8, 2017

Filed #81 which switches to the first folder approach; if you'd prefer to keep the common parent approach, I can switch it over to commonpath instead.

@rmccue
Copy link
Contributor Author

rmccue commented Sep 8, 2017

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

Successfully merging a pull request may close this issue.

2 participants