-
Notifications
You must be signed in to change notification settings - Fork 87
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
Return all installed DotNetSDK versions #79
Changes from all commits
7d57e5c
36ca39d
da34277
1eea571
5887126
b6353cb
8069139
9e91f9e
47e602b
f7573bc
afe8a1d
32a6f0b
5546230
7ab4ced
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,11 +15,10 @@ internal static class DotNetSdkLocationHelper | |
{ | ||
private static readonly Regex DotNetBasePathRegex = new Regex("Base Path:(.*)$", RegexOptions.Multiline); | ||
private static readonly Regex VersionRegex = new Regex(@"^(\d+)\.(\d+)\.(\d+)", RegexOptions.Multiline); | ||
private static readonly Regex SdkRegex = new Regex(@"(\S+) \[(.*?)]$", RegexOptions.Multiline); | ||
|
||
public static VisualStudioInstance GetInstance(string workingDirectory) | ||
{ | ||
string dotNetSdkPath = GetDotNetBasePath(workingDirectory); | ||
|
||
public static VisualStudioInstance GetInstance(string dotNetSdkPath) | ||
{ | ||
if (string.IsNullOrWhiteSpace(dotNetSdkPath)) | ||
{ | ||
return null; | ||
|
@@ -58,10 +57,20 @@ public static VisualStudioInstance GetInstance(string workingDirectory) | |
discoveryType: DiscoveryType.DotNetSdk); | ||
} | ||
|
||
private static string GetDotNetBasePath(string workingDirectory) | ||
public static IEnumerable<VisualStudioInstance> GetInstances(string workingDirectory) | ||
{ | ||
foreach (var basePath in GetDotNetBasePaths(workingDirectory)) | ||
{ | ||
var dotnetSdk = GetInstance(basePath); | ||
if (dotnetSdk != null) | ||
yield return dotnetSdk; | ||
} | ||
} | ||
|
||
private static IEnumerable<string> GetDotNetBasePaths(string workingDirectory) | ||
{ | ||
const string DOTNET_CLI_UI_LANGUAGE = nameof(DOTNET_CLI_UI_LANGUAGE); | ||
|
||
Process process; | ||
try | ||
{ | ||
|
@@ -83,12 +92,12 @@ private static string GetDotNetBasePath(string workingDirectory) | |
catch | ||
{ | ||
// when error running dotnet command, consider dotnet as not available | ||
return null; | ||
yield break; | ||
} | ||
|
||
if (process.HasExited) | ||
{ | ||
return null; | ||
yield break; | ||
} | ||
|
||
var lines = new List<string>(); | ||
|
@@ -109,10 +118,37 @@ private static string GetDotNetBasePath(string workingDirectory) | |
var matched = DotNetBasePathRegex.Match(outputString); | ||
if (!matched.Success) | ||
{ | ||
return null; | ||
yield break; | ||
} | ||
|
||
var basePath = matched.Groups[1].Value.Trim(); | ||
|
||
return matched.Groups[1].Value.Trim(); | ||
yield return basePath; // We return the version in use at the front of the list in order to ensure FirstOrDefault always returns the version in use. | ||
|
||
var lineSdkIndex = lines.FindIndex(line => line.Contains("SDKs installed")); | ||
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. I was worried about this English literal but it's forced to English
|
||
|
||
if (lineSdkIndex != -1) | ||
{ | ||
lineSdkIndex++; | ||
|
||
while (lineSdkIndex < lines.Count && !string.IsNullOrEmpty(lines[lineSdkIndex])) | ||
{ | ||
var sdkMatch = SdkRegex.Match(lines[lineSdkIndex]); | ||
|
||
if (!sdkMatch.Success) | ||
break; | ||
|
||
var version = sdkMatch.Groups[1].Value.Trim(); | ||
var path = sdkMatch.Groups[2].Value.Trim(); | ||
|
||
path = Path.Combine(path, version) + Path.DirectorySeparatorChar; | ||
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. I wasn't clear with what I meant on this—sorry about that. I meant that this is not a public-facing method, and when we use basePaths in GetInstance, we use Path.Combine to combine it with other things, so it doesn't matter if it has a slash at the end or not. I noticed the base path from 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. I see, but at the bottom in GetInstance we set dotNetSdkPath directly into new VisualStudioInstance(..) and the VisualStudioInstance constructor saves it directly to VisualStudioRootPath and MSBuildPath so that's where it would break the existing functionality having a trailing separator. 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. Ah, missed that. You're right. |
||
|
||
if (!path.Equals(basePath)) | ||
yield return path; | ||
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. Part of me wants to order this list by version descending ( 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. I think it's still useful to sort the results even with the basePath first because of #106. If the default SDK is from after the runtime in use, it'll be disqualified, and we'd go with the second in the list. If you don't clean your machine regularly and still have a 1.0 SDK lying around, that's what we'd find and register with RegisterDefaults. 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. I'm convinced. 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. So should I remove the new iterator/yield code and go back to the code using List with Reverse(), or do we have a better solution based on the current code? 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. I think that's the best solution. I already wrote up the change in #106, so I wouldn't worry about remaking that change here. |
||
|
||
lineSdkIndex++; | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
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.
nit:
GetDotNetBasePaths
could be an iterator method and simplyyield return basePath
here and thenyield return path
in the loop below. Just a thought, looks good either way.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 like this idea and have submitted a new commit with this change. Looks quite nice and clean.