-
Notifications
You must be signed in to change notification settings - Fork 323
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 for Multiple Content Roots #1821
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but @4e6 should have the final say on this one
31afa9d
to
befd664
Compare
@@ -2235,8 +2176,7 @@ addition of the root in order to inform them of the content root's ID. | |||
|
|||
```typescript | |||
{ | |||
id: UUID; // The content root ID | |||
absolutePath: [String]; | |||
root: ContentRoot; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a reminder to make sure that the appropriate task is scheduled on the IDE side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@farmaazon @MichaelMauderer I understand that this will likely need to be integrated for the https://github.com/enso-org/ide/issues/1634
Besides, I understand that before that the file/rootAdded
notification was not being handled. Is it a breaking change for the IDE that we will start sending it? Will it cause errrors if you have no handler for it? If yes, I'd like to ask if you can add at least a dummy handler for compatibility which can later be extended to its full functionality.
.../language-server/src/main/scala/org/enso/languageserver/filemanager/ContentRootManager.scala
Show resolved
Hide resolved
...uage-server/src/main/scala/org/enso/languageserver/filemanager/ContentRootManagerActor.scala
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed that the root manager doesn't try to access the project directory during the language server startup
now it uses the Manager
from old Config static content roots
Had to change the initialization logic slightly to avoid duplicating the initial set of roots; the external behaviour should remain the same
which are published by the RuntimeConnector on the eventStream
befd664
to
67826d2
Compare
Pull Request Description
Closes #1780
Important Notes
Checklist
Please include the following checklist in your PR: