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

Break direct C# dependency and instead light up debug/build functionality at runtime #1903

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

baronfel
Copy link
Contributor

@baronfel baronfel commented Jul 30, 2023

WHAT

🤖 Generated by Copilot at f225697

This pull request adds a feature to make the ms-dotnettools.csharp extension optional for debugging and building F# projects in VS Code. It introduces a new context variable fsharp.debuggerAvailable that depends on the availability and status of the C# extension, and uses it to conditionally activate various commands and shortcuts in the package.json file. It also adds a new module CSharpExtension that provides the logic for checking and warning about the C# extension, and modifies the Debugger and MSBuild modules to use it.

🤖 Generated by Copilot at f225697

To debug and build F# with ease
You no longer need C# keys
Just check fsharp.debuggerAvailable
And if not, show a warning label
And activate the commands as you please

🚨🔧🚀

WHY

Ok, so we have a nonzero amount of folks that just want to get going with F#, and right now our hard dependency on the C# extension can introduce a source of instability. What if we could detect the presence of the C# extension at run time and light up debug/launch tasks support if it is found, while notifying the user that those features will not be available until they install that extension?

Basic plan of attack:

  • make a new Context value for debugger support
  • set that value by probing for the C# extension
  • change debugger/msbuild activation to only register commands if C# is present
  • use the context value to act as part of the when condition for lots of the commands we declare in package.json

TODO:

  • manual testing
  • figure out how the user dialog when no C# extension is available should look
  • figure out how to detect the extensions' runtime context and swap between the openVSIX gallery C# extension and the VSCode marketplace C# extension
  • respond to new extension installations over the lifetime of the Ionide extension
  • ensure msbuild/debugger command registrations occur when C# is installed after activation
  • ensure solution and project-level right-click options only show when debugger is available
  • rename the context key as @TheAngryByrd suggests below

HOW

🤖 Generated by Copilot at f225697

  • Make the C# extension optional for debugging and building F# projects (link, link, link, link, link, link, link, link, link, link, link)
  • Add a condition to the when property of the MSBuild and F# commands in the package.json file, using the fsharp.debuggerAvailable context variable (link, link, link, link, link, link)
  • Remove the ms-dotnettools.csharp extension from the extensionDependencies array in the package.json file (link)
  • Add a new module CSharpExtension in src/Components/CSharpExtensionSupport.fs that defines functions and variables to check, set, and warn about the C# extension availability (link, link)
  • Modify the activate functions in the Debugger and MSBuild modules in src/Components/Debugger.fs and src/Components/MSBuild.fs to use the CSharpExtension module and only register the commands and providers if the C# extension is available (link, link)
  • Remove a trailing whitespace from the package.json file (link)

release/package.json Outdated Show resolved Hide resolved
src/Components/CSharpExtensionSupport.fs Outdated Show resolved Hide resolved
@baronfel baronfel changed the title Crazy idea to help F# folks getting started - break C# dependency Break direct C# dependency and instead light up debug/build functionality at runtime Aug 5, 2023
@baronfel baronfel marked this pull request as ready for review August 5, 2023 15:08
@baronfel baronfel marked this pull request as draft August 19, 2023 19:49
@baronfel baronfel marked this pull request as ready for review August 19, 2023 20:59
@baronfel
Copy link
Contributor Author

Ok, this is ready for other folks to poke at it. The build-related commands (restore, clean, build, rebuild) are always available, and the run commands are too. Only the debug commands are really disabled now, and the way we enforce this is by wrapping the call to generate the in-memory debug configurations. If no C# extension has been found, we generate no configurations. Otherwise we do our normal thing.

Comment on lines +31 to +61
let tryFindCSharpExtension () =
if not hasLookedForCSharp then
match extensions.getExtension resolvedCSharpExtensionName with
| None -> csharpAvailableContext false
| Some e ->
csharpExtension <- e
csharpAvailableContext true

hasLookedForCSharp <- true

hasCSharp

let warnAboutMissingCSharpExtension () =
if not hasWarned then
window.showWarningMessage (
$"The C# extension isn't installed, so debugging and some build tools will not be available. Consider installing the C# extension to enable those features.",
[| "Install C# Extension" |]
)
|> Promise.ofThenable
|> Promise.bind (fun c ->
if c = Some "Install C# Extension" then
commands.executeCommand ("extension.open", [| Some(box resolvedCSharpExtensionName) |])
|> Promise.ofThenable
else
Promise.empty)
|> Promise.catch (fun e ->
printfn $"Error installing C# extension: {Fable.Core.JS.JSON.stringify e}"
Promise.empty)
|> ignore<Fable.Core.JS.Promise<_>>

hasWarned <- true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two are designed to be cheap to call, so that components that rely on the debugger can match/call them inside processing loops naievely instead of having to be truly 'reactive'

Comment on lines +339 to +369
match CSharpExtension.tryFindCSharpExtension () with
| false ->
CSharpExtension.warnAboutMissingCSharpExtension ()
promise { return ResizeArray() }
| true ->
promise {
logger.Info("Evaluating launch settings configurations for %O", folder)
let projects = Project.getLoaded ()
let! msbuildTasks = tasks.fetchTasks (msbuildTasksFilter)

let tasks =
projects
|> List.collect (fun (p: Project) ->
[ let projectFile = node.path.basename p.Project

let buildTaskForProject =
msbuildTasks
|> Seq.tryFind (fun t ->
t.group = Some vscode.TaskGroup.Build && t.name = projectFile)
// emit configurations for any launchsettings for this project
match readSettingsForProject p with
| Some launchSettings ->
yield! configsForProject (p, launchSettings, buildTaskForProject)
| None -> ()
// emit a default configuration for this project if it is an executable
match defaultConfigForProject (p, buildTaskForProject) with
| Some p -> yield p
| None -> () ])

return ResizeArray tasks
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is all the same logic, just wrapped in a 'if debugging exists then .. else' block

None

fun _ -> f p

tasks.registerTaskProvider ("msbuild", msbuildBuildTaskProvider)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two functions were unused.

<Compile Include="fsharp.fs" />
</ItemGroup>
<Import Project="..\.paket\Paket.Restore.targets" />
<?xml version="1.0" encoding="utf-8"?>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No actual change here - just line endings maybe?

@baronfel
Copy link
Contributor Author

If you're going to take this for a test drive, start by uninstalling C# and then reloading your environment. Debugging shouldn't work at all, but building/launching should.
Then install C# - you should get a notification that debugging has been enabled. Now debugging should work.
If you remove C#, there's no actual change - because the extension isn't actually removed until you reload VSCode. This is the same flow as starting up and not finding C#, which you already covered.

@baronfel
Copy link
Contributor Author

@TheAngryByrd pointed out that the extension list query for "@category:debuggers F#" doesn't point directly to the C# extension (the vehicle for the functionality). We should reach out to that team to see what we can do.

Alternatively we could keep the commands enabled on our side, but have their implementations simply send the notification to install the C# extension directly.

@TheAngryByrd
Copy link
Member

We get some disappointing first generated launch.json.

{
    // Use IntelliSense to learn about possible attributes.
    // Hover to view descriptions of existing attributes.
    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
    "version": "0.2.0",
    "configurations": [
        {
            "name": ".NET Core Launch (console)",
            "type": "coreclr",
            "request": "launch",
            "WARNING01": "*********************************************************************************",
            "WARNING02": "The C# extension was unable to automatically decode projects in the current",
            "WARNING03": "workspace to create a runnable launch.json file. A template launch.json file has",
            "WARNING04": "been created as a placeholder.",
            "WARNING05": "",
            "WARNING06": "If the server is currently unable to load your project, you can attempt to",
            "WARNING07": "resolve this by restoring any missing project dependencies (example: run 'dotnet",
            "WARNING08": "restore') and by fixing any reported errors from building the projects in your",
            "WARNING09": "workspace.",
            "WARNING10": "If this allows the server to now load your project then --",
            "WARNING11": "  * Delete this file",
            "WARNING12": "  * Open the Visual Studio Code command palette (View->Command Palette)",
            "WARNING13": "  * run the command: '.NET: Generate Assets for Build and Debug'.",
            "WARNING14": "",
            "WARNING15": "If your project requires a more complex launch configuration, you may wish to",
            "WARNING16": "delete this configuration and pick a different template using the 'Add",
            "WARNING17": "Configuration...' button at the bottom of this file.",
            "WARNING18": "*********************************************************************************",
            "preLaunchTask": "build",
            "program": "${workspaceFolder}/bin/Debug/<insert-target-framework-here>/<insert-project-name-here>.dll",
            "args": [],
            "cwd": "${workspaceFolder}",
            "console": "internalConsole",
            "stopAtEntry": false
        },
        {
            "name": ".NET Core Attach",
            "type": "coreclr",
            "request": "attach"
        }
    ]
}

Doubt this is something we can do but it will be a concern for newer people trying to debug a project.

@TheAngryByrd
Copy link
Member

Also trying to run a project from the Solution Explorer fails:

debugging-icedtasks.mp4

Where it does seem to work in the current version.

@TheAngryByrd
Copy link
Member

TheAngryByrd commented Sep 14, 2023

Wonder if there's gonna be complications with #1927

@baronfel
Copy link
Contributor Author

good spot - I'm assuming #1927 will go first. If it does then I'll do the same checks I've done here for the debugging commands.

@baronfel
Copy link
Contributor Author

We get some disappointing first generated launch.json.

{
    // Use IntelliSense to learn about possible attributes.
    // Hover to view descriptions of existing attributes.
    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
    "version": "0.2.0",
    "configurations": [
        {
            "name": ".NET Core Launch (console)",
            "type": "coreclr",
            "request": "launch",
            "WARNING01": "*********************************************************************************",
            "WARNING02": "The C# extension was unable to automatically decode projects in the current",
            "WARNING03": "workspace to create a runnable launch.json file. A template launch.json file has",
            "WARNING04": "been created as a placeholder.",
            "WARNING05": "",
            "WARNING06": "If the server is currently unable to load your project, you can attempt to",
            "WARNING07": "resolve this by restoring any missing project dependencies (example: run 'dotnet",
            "WARNING08": "restore') and by fixing any reported errors from building the projects in your",
            "WARNING09": "workspace.",
            "WARNING10": "If this allows the server to now load your project then --",
            "WARNING11": "  * Delete this file",
            "WARNING12": "  * Open the Visual Studio Code command palette (View->Command Palette)",
            "WARNING13": "  * run the command: '.NET: Generate Assets for Build and Debug'.",
            "WARNING14": "",
            "WARNING15": "If your project requires a more complex launch configuration, you may wish to",
            "WARNING16": "delete this configuration and pick a different template using the 'Add",
            "WARNING17": "Configuration...' button at the bottom of this file.",
            "WARNING18": "*********************************************************************************",
            "preLaunchTask": "build",
            "program": "${workspaceFolder}/bin/Debug/<insert-target-framework-here>/<insert-project-name-here>.dll",
            "args": [],
            "cwd": "${workspaceFolder}",
            "console": "internalConsole",
            "stopAtEntry": false
        },
        {
            "name": ".NET Core Attach",
            "type": "coreclr",
            "request": "attach"
        }
    ]
}

Doubt this is something we can do but it will be a concern for newer people trying to debug a project.

I think if we can get the C# extension to add fsharp here then your search will work @TheAngryByrd. They are also trying to make an entirely new debug type labeled C# that is entirely dynamic (configuration for that is here that we might be able to copy from - this one is driving the 'f5 from nothing' experience for C# users.

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

Successfully merging this pull request may close these issues.

None yet

2 participants