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

Debug: make launching multi root folder aware #29245

Closed
bpasero opened this issue Jun 22, 2017 · 8 comments
Closed

Debug: make launching multi root folder aware #29245

bpasero opened this issue Jun 22, 2017 · 8 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues on-testplan workbench-multiroot Multi-root (multiple folders) issues
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Jun 22, 2017

Today we seem to collect the launch.json from the single root workspace here: https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/electron-browser/debugConfigurationManager.ts#L345

In a multi root environment we need to change this assumption. There are a couple of options:

  • just take the first folders configuration and show the others from a picker (e.g. add a "More..." command to the debug dropdown which opens a picker)
  • track the active file and then use that files workspace as default when checking for the config file (there is IHistoryService.getLastActiveWorkspaceRoot() to help out)
  • just treat all folders the same and collect configs from all folders and merge them into one list
@bpasero bpasero added debug Debug viewlet, configurations, breakpoints, adapter issues workbench-multiroot Multi-root (multiple folders) issues labels Jun 22, 2017
@bpasero
Copy link
Member Author

bpasero commented Jun 28, 2017

After talking to @dbaeumer here is an alternative proposal:

  • as a user you can configure launch configurations for a multi root workspace
  • this workspace level configuration is stored outside the workspace for the workspace (requires new configuration infrastructure)
  • we always scan all launch configurations from all folders that are opened and show the user this list with an action to promote one configuration to workspace level to be able to use it
  • we allow to reference a specific launch configuration from the workspace setting into one of the folders
  • we allow compound configurations to start multiple

@isidorn isidorn added this to the July 2017 milestone Jun 28, 2017
@isidorn
Copy link
Contributor

isidorn commented Jul 19, 2017

After discussing with @weinand we have a slight iteration on @bpasero proposal from above

  • You can not configure specific launch configurations for a MR workspace - we believe there is no use case for this and variables ${workspaceRoot} and others could not be resolved
  • You can configure how the launch experience behaves in a MR workspace by referencing configurations from folders in a workspace
  • A user gets into this experience by clicking on the gear icon, we open a virtual launch document and write everything top level into the .code-workspace file under the toplevel launch field (similar to the settings experience)
  • For simplicity, if the user did not explicitly configure the launch experience for the MR workspace we simply show the configurations for the first root. The first time the user clicks on the gear menu we would fill the MR launch file with the references to the first launch.json, this way it would also be a useful example for the user on how to configure this

Example .code-workspace file if I have a MR workspace with Client and Server folder.

{
	"id": "1500470713095",
	"folders": [
		"file:///Users/isidor/Development/Client",
		"file:///Users/isidor/Development/Server"
	],
	"settings": {},
	"launch": { 
              "Launch Client": "Client:Launch Program",
              "Launch Server": "Server:Launch Program",
              "Launch Both": ["Client:Launch Program", "Server:Launch Program"]
         }
}

Notice in the example above:

  • we allow to rename the launch configurations for clarity
  • we are referencing folder names only by the name, an idea is to do path matching from right to left since two folders can have the same name
  • The last launch is actually a composite which references a configuration in each root folder

Additional actions for promoting a configuration from folder level to MR level could be handled on top of this. One of the ideas it to play with the 'Add Configuration' button in this scenario.

@bpasero
Copy link
Member Author

bpasero commented Jul 19, 2017

@isidorn @weinand a couple of things to consider:

  • what we do for debug should also work for tasks
  • how do we guide a user to setup the actual launch/task configurations in case there are none? if a user starts fresh with folders that neither have launch nor task configs it seems like we need a way to educate the user that the actual configs go into the folder
  • I like the syntax with using the folder name as reference instead of having to introduce IDs for the folders
  • we should have nice warnings showing up in the editor if a target config cannot be found (it seems likely that a user renames a launch config and suddenly it will no longer be found in the MR config)

@isidorn
Copy link
Contributor

isidorn commented Jul 20, 2017

  • Our proposal in theory works well for tasks as well. Though we would need @dbaeumer to confirm
  • In case there are no launch.json then we simply give him the experience to configure the launch.json for the first workspace. That is a logical first step for the user and something that should be easy for us to start with
  • yes, will make it easier to read
  • yes - though might be tricky with the json schema

@bpasero
Copy link
Member Author

bpasero commented Jul 20, 2017

@isidorn maybe it would make sense to ask the user for which folder to create a launch config / task config in case there is none.

@isidorn
Copy link
Contributor

isidorn commented Jul 20, 2017

After discussing this in the standup we have decided to start with the MRU list of configs in the debug dropdown for the multi root workspace. The list would contain most recently used configurations from each launch.json and a 'show all' action which would allow to show all configuration in quick pick where it is easy to filter.

If there is no launch.json in any of the workspaces, clicking on the gear icon the user would be asked in which root to create launch.json.

The proposal form above (referencing configurations from roots) we will treat as step 2 and will be added on top.

@bpasero
Copy link
Member Author

bpasero commented Jul 20, 2017

@isidorn makes sense, I think on top of that we discussed:

  • to show the folder from which a config is from in the dropdown to avoid name clashes
  • to pick a fixed number of configs from one of the folders (the first?) if no MRU list exists yet

@weinand weinand removed their assignment Jul 25, 2017
@isidorn isidorn mentioned this issue Jul 25, 2017
3 tasks
@isidorn
Copy link
Contributor

isidorn commented Jul 26, 2017

All the first steps discussed here are done.
For the future we should add an advanced mechanism for referencing configuraitons from a MR config (as specified above). However we can cover that in a seperate issue.

Closing

@isidorn isidorn closed this as completed Jul 26, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues on-testplan workbench-multiroot Multi-root (multiple folders) issues
Projects
None yet
Development

No branches or pull requests

3 participants