Skip to content

Don't infer cwd from workspace folder when there's an explicit program - #4197

Merged
DanTup merged 3 commits into
masterfrom
improve-default-cwd
Oct 4, 2022
Merged

DanTup merged 3 commits into
masterfrom
improve-default-cwd

Conversation

@DanTup

@DanTup DanTup commented Oct 4, 2022

Copy link
Copy Markdown
Member

@helin24 does this solve the issue you had?

If there's a program we will try to infer a cwd from it, but if that fails, we should no longer use the workspace folder. We'll only use the workspace folder directly is there was no open file or program.

Then, if we still don't have one (which we won't for a special target that doesn't have an enclosing WorkspaceFolder) we'll look for a common ancestor for all workspace folders.

@DanTup DanTup added is enhancement An enhancement or improvement that should be listed in release notes but is not a bug fix. in debugging Relates to the debug adapter or process of running debug sessions labels Oct 4, 2022
@DanTup DanTup added this to the v3.52.0 milestone Oct 4, 2022
@DanTup
DanTup requested a review from helin24 October 4, 2022 11:40
Always use folder, since defaultCwd could also be relative.

@helin24 helin24 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the problem we were having! Thanks!

Comment thread src/extension/providers/debug_config_provider.ts Outdated
@DanTup
DanTup merged commit 226afe5 into master Oct 4, 2022
@DanTup
DanTup deleted the improve-default-cwd branch October 4, 2022 17:53
@maik-kluwe

maik-kluwe commented Nov 2, 2022 •

Copy link
Copy Markdown

Did you tests the change with opened workspace?
Because it breaks my current configuration:

{
            "name": "test_app",
            "request": "launch",
            "type": "dart",
            "program": "lib/main.dart",
            "flutterMode": "debug",
            "args": [
                "-d",
                "chrome",
            ],
}

If i remove the program it runs perfectly now.

@DanTup

DanTup commented Nov 2, 2022 •

Copy link
Copy Markdown
Member Author

@maik-kluwe can you provide more details about your exact setup? What do you mean by "opened workspace"? Do you mean a .code-workspace file? If so, is this launch configuration inside the .code-workspace or inside .vscode/launch.json inside one of the workspace folders?

If the config above is inside the .code-workspace file then it is expected that this will fail. The issue is that relative paths in files at the workspace level are ambiguous, for example:

{
	"folders": [{
			"path": "/Users/danny/Desktop/dart_sample"
		}, {
			"path": "/Users/danny/Desktop/dart_sample_2"
	}],
	"launch": {
		"version": "0.2.0",
		"configurations": [
			{
				"name": "Dart",
				"type": "dart",
				"request": "launch",
				"program": "bin/main.dart" // Is this path relatve to dart_sample or dart_sample_2 ?
			}
		]
	}
}

Although this worked before, it was using the current open file which is not guaranteed to be correct (if you opened a file from another workspace folder and pressed Run, it will behave differently, and it's not clear that was expected).

If you have launch configurations with relative paths in the workspace file, you should use variables to indicate which workspace folder they are relative to:

https://code.visualstudio.com/docs/editor/variables-reference#_variables-scoped-per-workspace-folder

For example:

{
	"folders": [{
			"path": "/Users/danny/Desktop/dart_sample"
		}, {
			"path": "/Users/danny/Desktop/dart_sample_2"
	}],
	"launch": {
		"version": "0.2.0",
		"configurations": [
			{
				"name": "Dart",
				"type": "dart",
				"request": "launch",
				"program": "bin/main.dart",
				"cwd": "${workspaceFolder:dart_sample_2}", // This uses the workspaceFolder path from dart_sample_2
			}
		]
	}
}

If you want it to run "whichever project the open file is in" then you should be able to use ${fileWorkspaceFolder} or ${workspaceFolder}.

If this isn't the issue you are seeing, please file a new issue and include specific details that will help me reproduce. Thanks!

@maik-kluwe

Copy link
Copy Markdown

Sorry about the missing information.

In my case i have created a .code-workspace file with just the name and paths of the projects in it:

{
	"folders": [
		{
			"path": "C:\\path_to_project\\project1"
		},
		{
			"path": "C:\\path_to_project\\project2"
		},
		{
			"path": "C:\\path_to_project\\some_package"
		}
	]
}

In each of the Projects (in my case project1 and project2) there is an .vscode\launch.json config with the configuration posted earlier.

My assumption was that with this setup dart should know the location of the cwd / currentWorkingDirectiory.

At the end:
@DanTup your tip with variables to indicate which workspace folder has helped. I was missing the "cwd": "${workspaceFolder}", in my launch.json. Now it works like before. Thanks!

@DanTup

DanTup commented Nov 2, 2022

Copy link
Copy Markdown
Member Author

@maik-kluwe you are correct - if the launch config is inside the project folder, this should work - I'll take a look shortly. Thanks!

@DanTup

DanTup commented Nov 2, 2022

Copy link
Copy Markdown
Member Author

I can repro this - have opened #4239 and will do some digging. Sorry for the bother!

@DanTup

DanTup commented Nov 2, 2022 •

Copy link
Copy Markdown
Member Author

@maik-kluwe can you switch to the pre-release (v3.53.20221102) Dart extension and verify it fixes your problem? If so I'll push it out to the stable version:

Dart/Flutter pre-release versions

@maik-kluwe

maik-kluwe commented Nov 2, 2022 •

Copy link
Copy Markdown

It works 👍 Thank you!

@DanTup

DanTup commented Nov 2, 2022

Copy link
Copy Markdown
Member Author

Great, thanks for confirming! I'll push it out to stable shortly.

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

Labels

in debugging Relates to the debug adapter or process of running debug sessions is enhancement An enhancement or improvement that should be listed in release notes but is not a bug fix.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants