Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upFix get_project_path for multi-directory workspaces #472
Conversation
scriptis
changed the title
Fix #471
Fix get_project_path for multi-directory workspaces
Dec 1, 2018
This comment has been minimized.
This comment has been minimized.
coveralls
commented
Dec 1, 2018
•
This comment has been minimized.
This comment has been minimized.
|
While this does fix the CWD of spawned language servers, LSP still seems to stop functioning entirely when working in a non-primary directory in a workspace... I'm going to leave this open for the moment, but keep poking around to see if I can fix anything else. To see the problem first-hand:
The problem itself seems to be intermittent, and sometimes resolves itself when you restart Sublime, add/remove projects from your workspace, etc. This PR fixes things enough of the time to be worth merging, so I'll probably open a different PR sometime later this week if I can find the root of the problem. |
plugin/core/workspace.py Outdated
rwols
added
the
bug
label
Dec 2, 2018
rwols
added this to the Release 0.8 milestone
Dec 2, 2018
This comment has been minimized.
This comment has been minimized.
|
What's the progress on this? |
This comment has been minimized.
This comment has been minimized.
|
Haven't had time to work on this the past week, should have it sorted today. Sorry about the wait. |
scriptis
added some commits
Dec 8, 2018
This comment has been minimized.
This comment has been minimized.
|
Look good? @rwols |
rwols
merged commit b8109b1
into
tomv564:master
Dec 8, 2018
This comment has been minimized.
This comment has been minimized.
|
Thanks for the contribution, but I will revert this change now in favour of a full solution. The existing behaviour was not a "logic error", the function even documented that it would return the first folder. (Also see the note in the troubleshooting section and issue #33 linked from it) I'm sure we can make multi-folder projects work reliably, but if we need to look closer at the usages of |
added a commit
that referenced
this pull request
Dec 10, 2018
tomv564
referenced this pull request
Dec 10, 2018
Merged
Revert "Fix get_project_path for multi-directory workspaces" #478
added a commit
that referenced
this pull request
Dec 10, 2018
This comment has been minimized.
This comment has been minimized.
|
LSP should probably start communicating the current limitation by warning users and refusing server startup when files under the 2nd folder are opened. |
This comment has been minimized.
This comment has been minimized.
|
@tomv564: I don't follow your reasoning in reverting this. For workspaces with more than one active project, this evaluates to true, always returning the first directory of the workspace, even though the function is documented to return Further, as far as I am aware, this PR fixes the issues regarding LSP behavior in a multi-directory workspace. This was, after all, the entire point... |
This comment has been minimized.
This comment has been minimized.
|
The But perhaps this is a good time to look at supporting multi-directory workspaces: Say you have a window with two python projects:
Server sessions are shared per window
This is fixable with the cost of looking up the session by window root folder.
Support configuration per folder? A follow-up consideration: If users can have different sessions for |
scriptis commentedDec 1, 2018
Fixes a logic error that causes
get_project_pathto always return the first directory in the current workspace.