-
-
Notifications
You must be signed in to change notification settings - Fork 286
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
baronfel
wants to merge
18
commits into
main
Choose a base branch
from
break-dependency-on-csharp
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
cdbb771
first pass at detecting csharp
baronfel f225697
warning message
baronfel e969eeb
choose different C# extension based on execution state
baronfel 2891cc7
listen to extension changes to make sure enablement works
baronfel a86b015
markdown link to extension gallery
baronfel aa18afb
fix MS c# extension name
baronfel 683dade
fix hookup
baronfel 6a15bf4
bump structured logger dependency
baronfel ed848c4
corerctly link to extension install page for prompt
baronfel 3c48c3f
let users know when debugging functionality lights up
baronfel ff72781
Merge branch 'main' into break-dependency-on-csharp
baronfel 7d7a53c
Merge branch 'main' into break-dependency-on-csharp
baronfel 52407fe
fix after merge
baronfel c5340d3
rename context key
baronfel 2c0e40a
disable more commands when debugging is missing
baronfel ba0db66
reenable build commands, and make debug commands more reactive
baronfel 7d132bf
Merge branch 'main' into break-dependency-on-csharp
baronfel f310d33
Merge branch 'main' into break-dependency-on-csharp
baronfel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,5 @@ Fake.IO.Zip | |
Fake.Api.GitHub | ||
Fake.Tools.Git | ||
Fake.JavaScript.Yarn | ||
Octokit | ||
|
||
Octokit | ||
MSBuild.StructuredLogger |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,51 +1,51 @@ | ||
source https://api.nuget.org/v3/index.json | ||
storage: none | ||
framework: netstandard2.0 | ||
|
||
nuget Fable.Core | ||
nuget Fable.Promise | ||
nuget Fable.Node | ||
nuget Fable.Browser.Dom | ||
|
||
nuget Fable.HtmlConverter | ||
|
||
nuget Thoth.Json 7.0 | ||
|
||
git https://github.com/ionide/ionide-fsgrammar.git master | ||
|
||
github ionide/ionide-vscode-helpers:8e81bc03f11f07b8e0811b3d4598eadc78f32f2f src/Fable.Import.VSCode.fs | ||
github ionide/ionide-vscode-helpers:8e81bc03f11f07b8e0811b3d4598eadc78f32f2f src/Helpers.fs | ||
github ionide/ionide-vscode-helpers:8e81bc03f11f07b8e0811b3d4598eadc78f32f2f src/Fable.Import.Showdown.fs | ||
github ionide/ionide-vscode-helpers:8e81bc03f11f07b8e0811b3d4598eadc78f32f2f src/Fable.Import.VSCode.LanguageServer.fs | ||
|
||
group fsac | ||
source https://api.nuget.org/v3/index.json | ||
storage: packages | ||
framework: netstandard2.0 | ||
nuget fsautocomplete | ||
|
||
group build | ||
source https://api.nuget.org/v3/index.json | ||
storage: none | ||
framework: net7.0 | ||
|
||
nuget Fake.Core.Target | ||
nuget Fake.Core.Process | ||
nuget Fake.Core.ReleaseNotes | ||
nuget Fake.Core.Environment | ||
nuget Fake.Core.UserInput | ||
nuget Fake.DotNet.Cli | ||
nuget Fake.DotNet.AssemblyInfoFile | ||
nuget Fake.DotNet.Paket | ||
nuget Fake.DotNet.MsBuild | ||
nuget Fake.IO.FileSystem | ||
nuget Fake.IO.Zip | ||
nuget Fake.Api.GitHub | ||
nuget Fake.Tools.Git | ||
nuget Fake.JavaScript.Yarn | ||
nuget Octokit <= 0.49.0 | ||
|
||
// Enforce updated binlog parser until Fake gets an update, so we're not stuck | ||
// on an old sdk version. | ||
// ref: https://github.com/fsprojects/FAKE/issues/2744 | ||
nuget MSBuild.StructuredLogger >= 2.1.784 | ||
source https://api.nuget.org/v3/index.json | ||
storage: none | ||
framework: netstandard2.0 | ||
nuget Fable.Core | ||
nuget Fable.Promise | ||
nuget Fable.Node | ||
nuget Fable.Browser.Dom | ||
nuget Fable.HtmlConverter | ||
nuget Thoth.Json 7.0 | ||
git https://github.com/ionide/ionide-fsgrammar.git master | ||
github ionide/ionide-vscode-helpers:8e81bc03f11f07b8e0811b3d4598eadc78f32f2f src/Fable.Import.VSCode.fs | ||
github ionide/ionide-vscode-helpers:8e81bc03f11f07b8e0811b3d4598eadc78f32f2f src/Helpers.fs | ||
github ionide/ionide-vscode-helpers:8e81bc03f11f07b8e0811b3d4598eadc78f32f2f src/Fable.Import.Showdown.fs | ||
github ionide/ionide-vscode-helpers:8e81bc03f11f07b8e0811b3d4598eadc78f32f2f src/Fable.Import.VSCode.LanguageServer.fs | ||
group fsac | ||
source https://api.nuget.org/v3/index.json | ||
storage: packages | ||
framework: netstandard2.0 | ||
nuget fsautocomplete | ||
group build | ||
source https://api.nuget.org/v3/index.json | ||
storage: none | ||
framework: net7.0 | ||
nuget Fake.Core.Target | ||
nuget Fake.Core.Process | ||
nuget Fake.Core.ReleaseNotes | ||
nuget Fake.Core.Environment | ||
nuget Fake.Core.UserInput | ||
nuget Fake.DotNet.Cli | ||
nuget Fake.DotNet.AssemblyInfoFile | ||
nuget Fake.DotNet.Paket | ||
nuget Fake.DotNet.MsBuild | ||
nuget Fake.IO.FileSystem | ||
nuget Fake.IO.Zip | ||
nuget Fake.Api.GitHub | ||
nuget Fake.Tools.Git | ||
nuget Fake.JavaScript.Yarn | ||
nuget Octokit <= 0.49.0 | ||
// Enforce updated binlog parser until Fake gets an update, so we're not stuck | ||
// on an old sdk version. | ||
// ref: https://github.com/fsprojects/FAKE/issues/2744 | ||
nuget MSBuild.StructuredLogger >= 2.1.784 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
namespace Ionide.VSCode.FSharp | ||
|
||
open Fable.Import.VSCode | ||
open Fable.Import.VSCode.Vscode | ||
|
||
module CSharpExtension = | ||
|
||
let private msCSharpExtensionName = "ms-dotnettools.csharp" | ||
let private openvsixCSharpExtensionName = "muhammad-sammy.csharp" | ||
|
||
let private resolvedCSharpExtensionName = | ||
if env.appName = "Visual Studio Code" then | ||
msCSharpExtensionName | ||
else | ||
openvsixCSharpExtensionName | ||
|
||
let mutable private hasLookedForCSharp = false | ||
let mutable private hasCSharp = false | ||
let mutable private csharpExtension: Extension<obj> = null | ||
let mutable private hasWarned = false | ||
|
||
let private csharpAvailableContext: bool -> unit = | ||
let fn = Context.cachedSetter "fsharp.debugger.available" | ||
|
||
fun value -> | ||
hasCSharp <- value | ||
fn value | ||
|
||
let isCSharpAvailable () = hasCSharp | ||
|
||
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 | ||
|
||
let private notifyUserThatDebuggingWorks () = | ||
window.showInformationMessage ( | ||
$"The C# extension is installed, so debugging and build tools are now available for F# projects." | ||
) | ||
|> ignore<Thenable<_>> | ||
|
||
let activate (context: ExtensionContext) = | ||
// when extensions are installed or removed we need to update our state for the C# extension | ||
// so enablement/disablement works correctly | ||
context.Subscribe( | ||
extensions.onDidChange.Invoke(fun _ -> | ||
let previousCSharpValue = hasCSharp | ||
hasLookedForCSharp <- false | ||
let currentCSharpValue = tryFindCSharpExtension () | ||
|
||
match previousCSharpValue, currentCSharpValue with | ||
| false, true -> notifyUserThatDebuggingWorks () | ||
| true, false -> | ||
hasWarned <- false | ||
warnAboutMissingCSharpExtension () | ||
| _ -> () | ||
|
||
None) | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -336,34 +336,39 @@ module Debugger = | |
{ new DebugConfigurationProvider with | ||
override x.provideDebugConfigurations(folder: option<WorkspaceFolder>, token: option<CancellationToken>) = | ||
let generate () = | ||
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 | ||
} | ||
|
||
generate () // this bix/unbox is a hack because JS types | ||
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 | ||
} | ||
Comment on lines
+339
to
+369
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
generate () // this box/unbox is a hack because JS types | ||
|> box | ||
|> unbox | ||
|
||
|
@@ -386,6 +391,7 @@ module Debugger = | |
ProviderResult.Some(U2.Case1 debugConfiguration) } | ||
|
||
let activate (c: ExtensionContext) = | ||
|
||
commands.registerCommand ("fsharp.runDefaultProject", (buildAndRunDefault) |> objfy2) | ||
|> c.Subscribe | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -486,19 +486,6 @@ module MSBuild = | |
let registerCommand com (action: unit -> _) = | ||
commands.registerCommand (com, action |> objfy2) |> context.Subscribe | ||
|
||
let registerCommand2 com (action: obj -> obj -> _) = | ||
commands.registerCommand (com, action |> objfy3) |> context.Subscribe | ||
|
||
/// typed msbuild cmd. Optional project and msbuild host | ||
let typedMsbuildCmd f projOpt = | ||
let p = | ||
if JS.isDefined projOpt then | ||
Some(unbox<string> (projOpt)) | ||
else | ||
None | ||
|
||
fun _ -> f p | ||
|
||
tasks.registerTaskProvider ("msbuild", msbuildBuildTaskProvider) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These two functions were unused. |
||
|> context.Subscribe | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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'