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

multi-root workspaces #170

Open
nadako opened this issue Sep 7, 2017 · 27 comments
Open

multi-root workspaces #170

nadako opened this issue Sep 7, 2017 · 27 comments

Comments

@nadako
Copy link
Member

nadako commented Sep 7, 2017

The multi-root workspaces feature is coming soon-ish to VS Code (already in Insiders builds) and we'll have add support for it to be ready.

The API is already there and it works for current single-folder workspaces, so we can start now.

There are several things affected:

  • haxe executable and display arguments configuration (might need to change the vshaxe ext API)
  • dependency explorer (cache per folder and update the view depending on currently opened file?)
  • language server (start a server per-folder, gotta check out the latest ls-client and proposed protocol changes)

See also: https://github.com/Microsoft/vscode/wiki/Extension-Authoring:-Adopting-Multi-Root-Workspace-APIs

@nadako
Copy link
Member Author

nadako commented Sep 7, 2017

@nadako
Copy link
Member Author

nadako commented Sep 21, 2017

I think we should start with figuring out our extension API with regard to multi-root workspaces.

There are two things that are affected by this: haxeExecutable and registerDisplayArgumentsProvider(name, provider).

We can change that to something like getHaxeExecutable(rootUri) and registerDisplayArgumentsProvider(rootUri, name, provider).

But I'm thinking a more future-proof solution would be to have something like getWorkspaceFolderApi(rootUri) that then provides access to haxeExecutable and registerDisplayArgumentsProvider.

What's not quite clear to me is how to properly deal with workspace folder addition/deletion here, e.g. whether we can guarantee that getWorkspaceFolderApi will be ready for third-party extensions if they just subscribe to onDidChangeWorkspaceFolders. Gotta check out vscode event handler ordering spec.

What do you think @Gama11?

@nadako
Copy link
Member Author

nadako commented Sep 21, 2017

Internally, I'm also thinking about some abstraction for "workspace folder" that contains executable and arguments configuration and manages language server instance. Then we just create/destroy these objects when user adds/removes workspace folders. Then other, view-level thingies like status bar selector, dependency view, etc access APIs for current workspace folder.

@Gama11
Copy link
Member

Gama11 commented Sep 21, 2017

Makes sense to me. :) (both comments)

@nadako
Copy link
Member Author

nadako commented Nov 3, 2017

VS Code 1.18 will add finally support for multi-root workspaces in stable releases, so we gotta accelerate this.

@remcohuijser
Copy link

Hi guys,

I really could use this feature as we usually work with both a code project and a test project (that tests part of the code) in the same folder. In HaxeDevelop you need to have 2 editors open but now with vsc you could very nicely show both in a single editor.

What I am currently missing is that the extension does not look in all workspace folders for an hxml file. Can this be added?

@nadako
Copy link
Member Author

nadako commented Nov 18, 2017

We're in the middle of refactoring the code so it can be used in a multi-root environment.

@remcohuijser
Copy link

Thanks Dan: that is very good to hear. It there any way we can contribute to this? We have been using Haxe for more than 5 years now and I think that the move to VSC is a sensible one, even though Flash/HaxeDevelop has served us well.

There are a couple of important things missing though: what do you recon is the best way to share this kind of wishlist?

@cmandlbaur
Copy link

Has there been any progress on this?

Holding out on trying VSHaxe in earnest until this is supported as I have a large project and would need completion isolated to a subproject and maybe some other related projects, but not globally.

Keen to possibly help out where I can.

@Gama11
Copy link
Member

Gama11 commented Jun 14, 2018

Not recently, no, we've been busy designing the new display protocol for Haxe 4 (and probably will be for a while). @nadako had a branch for multi-root support here: https://github.com/vshaxe/vshaxe/tree/multiroot

Not sure I understand why you need "completion isolated" though?

@cmandlbaur
Copy link

cmandlbaur commented Jun 14, 2018

Perhaps I don't fully understand what MultiRoot buys me.

What I currently get in IntelliJ is:
Project

-> ModuleA
  -> src
  -> testSrc
-> ModuleB
  -> src
  -> testSrc

With this I can set it up such that ModuleA has access to code in ModuleB, but ModuleB doesn't have access to ModuleA. Additionally code in src does not have access to testSrc, while testSrc dirs have access to src.

Would I be able to get this currently or do I require multi-root workspaces?

@nadako
Copy link
Member Author

nadako commented Jun 14, 2018

Multi-root workspace basically means different configurations for different root folders in the same workspace (aka window). For Haxe extension this would mean that different completion/build configuration is used based on the open file.

@nadako
Copy link
Member Author

nadako commented Jun 14, 2018

So in your example, yes, you can use different configurations for ModuleA and ModuleB, but I don't think you can use different configurations for folders inside a single root. It would be interesting to research this though, as test source folders is a very common thing.

@cmandlbaur
Copy link

Thanks @nadako! Super keen for this!

@nadako
Copy link
Member Author

nadako commented Jun 14, 2018

I should really finish this one...

@cmandlbaur
Copy link

@nadako what is still outstanding on the multiroot branch? How can I help?

@nadako
Copy link
Member Author

nadako commented Jul 30, 2018

I'm pretty sure the multiroot branch is now outdated after all the recent-ish changes and we gotta continue from master. I actually got some refactoring into master before going MIA, but unfortunately I don't have a lot of time nowadays...

Anyway, the main idea is that we should split "model" stuff from "presentation" more and have a top-level "workspace context" object that manages all the models we have (language server instance, dependency explorer, build configurations, etc). And then we need to make "presentation" things (the code that shows status bar items, panels, etc) to be able to display their data from different workspace contexts. This raises some questions about choosing the "current" context to display, since VS Code itself doesn't have this concept, but I think that simply selecting the context based on the currently focused editor will work pretty well and we can figure out the details.

At this point we also will need to rework the third-party extension API somehow in a compatible way :)

@cmandlbaur
Copy link

Will try take a stab at it and see how far I can get.

@cmandlbaur
Copy link

Quick update on this.

I have hacked together something that kinda works, but I had to refactor huge swathes of code and I'm not sure that I haven't broken more than I've fixed.

Something that I've also noticed is there are a few commands that the server project is listening for directly that we would need to move out to disambiguate which language server instance a change is associated with.

That said I've this has been a good dive into the code. I will try look for other issues I can pick up here and there when I get a gap so that maybe we can move towards supporting this over time. Additionally it seems like main vshaxe project might benefit from some dependency injection, would this be something you'd be interested in? If so which framework?

@Gama11 , @nadako

@Gama11
Copy link
Member

Gama11 commented Aug 3, 2018

That makes sense, I would definitely have expected that some changes to the language server are necessary.

I don't really see a pressing need for dependency injection. Each library / dependency we add comes with a maintenance cost - we already have quite a few, at least in the language server.

It might be a good idea to do this step-by-step, instead of trying to add multi-root support to everything at once? Some features should be relatively isolated from each other, such as tasks and completion (which should make that easier / possible in the first place). I'll let @nadako comment on that though, since he probably has a better grasp on that, already having done part of the implementation.

@nadako
Copy link
Member Author

nadako commented Aug 3, 2018

Yes, please let's not use DI framework for vshaxe. Not only that Haxe DI frameworks are pretty badly written from what I seen, but DI frameworks in general are harmful, in my opinion. Yes, transient dependencies can be annoying to write, but it's a price I'd pay any day for the well-structured code with clear dependencies. Anyway, this is off-topic now :-P

Regarding the actual work, my goal is/was to gradually refactor things to make them work with a specific workspace folder passed to them, and just always pass workspace.workspaceFolders[0] for now. Then, when everything lose their dependency on the global workspace.rootPath, introduce some nice "workspace folder context" abstraction that would create and manage things for specific folders and then a manager class that would create these context instances and have some notion of the "current" one, so presentation classes can display info from the current workspace folder.

@nadako
Copy link
Member Author

nadako commented Aug 3, 2018

(PS we have a lot of dependency injection in vshaxe and haxe-ls, pls dont confuse frameworks that provide magic with the principle itself)

@cmandlbaur
Copy link

Pretty much moved everything to point to a ‘WorkspaceFolderModel’.

I will peel back a few of the changes and try lay out a step by step here before submitting a PR.

What I use for unit testing and mocking?

@cmandlbaur
Copy link

I have pushed the basic changes to support looping through workspace folders with only the root workspace folder being used right now.

Can take a look here and let me know if there's anything glaringly bad, otherwise will submit a PR in a day or two.

@cmandlbaur
Copy link

@nadako any guidance please?

I am happy to rebase on latest and get things going again, just need to know if its in the right direction.

@varadig
Copy link

varadig commented Sep 24, 2021

What is the status of this feature?

@47rooks
Copy link

47rooks commented Feb 23, 2024

I'm starting to use multi-root workspaces a bit. So this would be really good to have. I'm using lime but they are waiting on vshaxe supporting multi-root before updating their extension. Any idea when this might be available ?

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

No branches or pull requests

6 participants