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

[dtd] Add a 'getProjectRoots' method on the DTD file system API #54790

Closed
kenzieschmoll opened this issue Jan 31, 2024 · 10 comments
Closed

[dtd] Add a 'getProjectRoots' method on the DTD file system API #54790

kenzieschmoll opened this issue Jan 31, 2024 · 10 comments
Labels
area-tools A meta category for issues that should be addressed by tooling (prefer more concrete areas). triaged Issue has been triaged by sub team

Comments

@kenzieschmoll
Copy link
Contributor

This should return the value set by the 'setProjectRoots' method (which requires a secret). The 'getProjectRoots' should not require a secret.
@CoderDake @bkonyi

@kenzieschmoll kenzieschmoll added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. vm-debugger labels Jan 31, 2024
@a-siva
Copy link
Contributor

a-siva commented Feb 1, 2024

Why is this a 'area-vm' issue, can this be handled purely in DTD code ?

@a-siva a-siva added needs-info We need additional information from the issue author (auto-closed after 14 days if no response) triaged Issue has been triaged by sub team area-tools A meta category for issues that should be addressed by tooling (prefer more concrete areas). and removed area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. needs-info We need additional information from the issue author (auto-closed after 14 days if no response) vm-debugger labels Feb 1, 2024
@a-siva
Copy link
Contributor

a-siva commented Feb 1, 2024

chatted with @bkonyi and updated the labels.

@DanTup
Copy link
Collaborator

DanTup commented Feb 1, 2024

getProjectRoots

Out of interest, do we really want the project roots here, or will the workspace roots do?

If the goal is to restrict where the clients can read from, I think the workspace (eg. the folders the user opened in their IDE) might make more sense. However these are not necessarily project roots (because they may have opened a monorepo).

Currently, in Dart-Code we have to walk the tree to find projects (where there are pubspec.yamls) to send for PubRootDirectories, but maybe we should just report workspace folders to DTD and it could find the projects (to avoid each IDE/tool doing the same walking, and maybe doing it slightly differently)?

@kenzieschmoll
Copy link
Contributor Author

What is the difference between "project roots" and "workspace roots"? I have been using those terms interchangeably. Is a "project root" considered to be a directory that contains a pubspec.yaml file and a "workspace root" is a top level directory in the IDE?

If so, then I'm referring to workspace roots in this issue.

@kenzieschmoll
Copy link
Contributor Author

CC @CoderDake @bkonyi @helin24 - maybe we should come to agreement on terminology here. Should we change setProjectRoots and getProjectRoots to be setWorkspaceRoots and getWorkspaceRoots?

@DanTup
Copy link
Collaborator

DanTup commented Feb 2, 2024

What is the difference between "project roots" and "workspace roots"? I have been using those terms interchangeably. Is a "project root" considered to be a directory that contains a pubspec.yaml file and a "workspace root" is a top level directory in the IDE?

Yeah, that's how I'd describe them. VS Code and LSP call the top-level folders you open (which can be one - a single-root workspace, or multiple - a multi-root workspace) "WorkspaceFolders". I use "project" to mean something with a pubspec (maybe "package" would be better for Dart, but in other languages they'd be Projects).

We also often refer to "analysis roots" in the analyzer, but for VS Code/LSP these are just the same as "Workspace Folders"/"Workspace roots" (with some uninteresting exceptions).

@CoderDake
Copy link

SGTM :)

@DanTup
Copy link
Collaborator

DanTup commented Feb 5, 2024

This came up earlier and @jacob314 informed me that setPubRootDirectories doesn't actually need the individual project folders, and it would be better to send the IDE workspace roots.

He also pointed out that Pub has a concept of "Workspaces" and the term is a little overloaded.

So, it might be better to label these roots discussed above consistently as "IDE Workspace Roots" to avoid any confusion with "Pub Workspace Roots" that might exist in future. Although those might end up being the same on most cases, it's not guaranteed so it may bet best to avoid the ambiguous "Workspace Roots" without a qualifier.

@CoderDake
Copy link

@helin24
Copy link
Contributor

helin24 commented Feb 5, 2024

And this should work fine for IntelliJ also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-tools A meta category for issues that should be addressed by tooling (prefer more concrete areas). triaged Issue has been triaged by sub team
Projects
None yet
Development

No branches or pull requests

5 participants