lahodaj commented on PR #7610: URL: https://github.com/apache/netbeans/pull/7610#issuecomment-2263611724
> Thank you @lahodaj for reviewing the PR. > > > So, what if, just before the new code here, we would check if there's exactly one workspace, and if there is, we would use the single workspace. (Keeping the pre-existing code computing the workspace folders, as that is needed as a default working directory.) > > Having one workspace should be the typical thing. > > Yes, having one workspace is the typical use-case. Note that the current default is that for every workspace/VS Code UI window, there is a separate backend started. I.e. this is not about the number of windows open, but rather whether the backend is shared by multiple windows, which is not the default. > > > What do you think? > > When I was adding this new code, my concern with this approach of defaulting to the single open workspace was that it may lead to a security/_cross-talk_ issue if someone opens a file external to the regular workspace (with workspace folders). > > * The workspace arguments and VM options might contain credentials etc. which should not be sent to the workspace-external program, _even when its a trusted file_. Isn't that a problem with this patch as well? If the backend is shared among multiple windows, and I have multiple windows with workspaces without folders, this patch will pick one of the UI windows/settings/clients semi-"randomly", no? > > * Additionally, opening the same file when another workspace is open would lead to different runtime behaviour. > > * This would also be outside the user's control. I suspect the same will be with this patch - if the backend is shared among windows, then one of then will be selected, no? And if the backend is not shared, then there's only one workspace, and all should work consistently? > > * On the other hand, by using only the _global_ workspace, i.e. without any workspace folders, the user would have greater control. > > * It would be clear that only the global run configuration is in effect. > > > Do you think that in case no suitable workspace is found, then this method should introduce adding a new `Workspace`? > > * I think this might be harder to achieve since it would require changing `WorkspaceServiceImpl` and maybe `WorkspaceService` and the scope of that seems very wide. The Workspace is basically just a part of the mirror that mirrors the UI Window. I don't think we can meaningfully create a new one. > > > Please let me know what you think. > > Thank you. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
