-
Notifications
You must be signed in to change notification settings - Fork 676
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
Adding folders with Miscellaneous Cs files as Launch Targets #2471
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2471 +/- ##
==========================================
+ Coverage 64% 64.41% +0.41%
==========================================
Files 90 90
Lines 4114 4120 +6
Branches 590 592 +2
==========================================
+ Hits 2633 2654 +21
+ Misses 1312 1291 -21
- Partials 169 175 +6
Continue to review full report at Codecov.
|
looks good to me |
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.
I doubt that customers would understand when they would want select a launch target for "Miscellaneous" and what effect it would have. I'm also suspicious of the "CSX" and "Cake" launch targets. Why do I need three launch targets that do the same thing? (Note that the LaunchTarget.kind
field is never used anywhere.)
Should we just consolidate all of these into a single launch target for the root folder?
cc @filipw about the CSX launch target
@@ -170,6 +177,16 @@ function resourcesToLaunchTargets(resources: vscode.Uri[]): LaunchTarget[] { | |||
kind: LaunchTargetKind.Cake | |||
}); | |||
} | |||
|
|||
if (hasCs && !hasSlnFile && !hasCsProjFiles && !hasProjectJson && !hasProjectJsonAtRoot) { |
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.
Having a separate launch target called "Miscellaneous" seems a little weird to me from a customer perspective. Wouldn't it be better if we just re-purpose the launch target here: https://github.com/OmniSharp/omnisharp-vscode/pull/2471/files#diff-0f649e373df5c5e30b19fa598a56b55bR149? If there are any C# files found but none of those conditions are met, we still want to be able to launch on the root folder.
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.
+1,
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.
Thanks!
@DustinCampbell I have removed the Misc file launch target and using the root folder as the launch target now. Does that seem right ? |
Yes, that seems right to me. I added @filipw as a reviewer to see if he has any concerns. |
e1db81f
to
4bd398c
Compare
Fixes: #47
With the effort to support Miscellaneous files in O#(OmniSharp/omnisharp-roslyn#1252), modifying the LaunchTargets to add a folder with Misc files, if it found a cs file with no sln or csproj files