-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add Executable launch profile support to dotnet run and dotnet watch #51727
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
base: release/10.0.2xx
Are you sure you want to change the base?
Add Executable launch profile support to dotnet run and dotnet watch #51727
Conversation
baronfel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Good initial attempt, but we can clean this up some. Please address my review comments.
src/Cli/dotnet/Commands/Run/LaunchSettings/ExecutableLaunchSettingsProvider.cs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| private static ProjectLaunchSettingsModel? TryGetLaunchProfileSettings(string projectDirectory, string projectNameWithoutExtension, string appDesignerFolder, BuildOptions buildOptions, string? profileName) | ||
| private static LaunchSettingsModel? TryGetLaunchProfileSettings(string projectDirectory, string projectNameWithoutExtension, string appDesignerFolder, BuildOptions buildOptions, string? profileName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dotnet/dotnet-testing if we expand the 'launch settings' handlers that are known, how should the testing experience react? does test support launch settings profiles today?
|
@baronfel I just read in diagonal the PR. I think it'd be great to ensure that's also working for test verb. cc @Youssef1313 |
Co-authored-by: baronfel <[email protected]>
- Created base LaunchSettingsModel with ProfileKind discriminator - Derived ProjectLaunchSettingsModel and ExecutableLaunchSettingsModel - Added JSON binding models for deserialization - Updated providers to use JsonSerializer.Deserialize instead of manual traversal - Exposed SupportedProfileTypes from LaunchSettingsManager - Updated LaunchSettingsProfile.cs to use array of supported types with comment - Updated RunCommand to use pattern matching on ProfileKind - Split Execute method into ExecuteWithProjectProfile and ExecuteWithExecutableProfile - Updated all method signatures to use base LaunchSettingsModel type Co-authored-by: baronfel <[email protected]>
Changed hardcoded error message to use CliCommandStrings.LaunchProfileHandlerCannotBeLocated for proper localization support. Co-authored-by: baronfel <[email protected]>
Simplified argument building logic to make it clearer that ApplicationArgs are properly escaped before being combined with launchSettings.CommandLineArgs. Added comments explaining that CommandLineArgs from JSON cannot be reliably parsed into tokens. Co-authored-by: baronfel <[email protected]>
Extracted the common code from ExecuteWithProjectProfile and ExecuteWithExecutableProfile into a new ExecuteCommand method. This method handles: - Applying launch settings to the command - Applying environment variables from command line - Sending telemetry - Setting up Ctrl-C handler - Executing the command This reduces code duplication and makes the execution flow clearer. Co-authored-by: baronfel <[email protected]>
Added tests to verify that the executable command handler correctly applies: - Environment variables from launch profile - Working directory settings - Command line arguments combination (profile + command line) - Environment variable expansion in executable paths Created new test assets: - AppForExecutableProfile: Helper app to verify environment and arguments - AppWithDetailedExecutableProfile: Test project with various executable profiles All tests verify the correct application of launch profile settings to the generated command. Co-authored-by: baronfel <[email protected]>
33d9898 to
1c8a35c
Compare
1c8a35c to
8cfe2ea
Compare
8cfe2ea to
6b63ed2
Compare
|
What should For example, if I specify: {
"profiles": {
"Execute": {
"commandName": "Executable",
"executablePath": "dotnet",
"commandLineArgs": "--version"
}
}
}Should it just run |
I think the same question applies for Now, this being said, I agree that if you do "conflicting" queries that can lead to be situations and I don't know what should be the proper UX here. I still tend to think I'd prefer to have something (good or bad) similar to |
|
The difference is that |
Refactors launch profile parsing and adds support for
Executablecommand name.Fixes environment variable expansion and adds tests.