Address TODOs

Author: dsplaisted

src/Cli/dotnet/Commands/Tool/Execute/ToolExecuteCommand.cs

@@ -33,20 +33,15 @@ internal class ToolExecuteCommand(ParseResult result, ToolManifestFinder? toolMa
     private readonly string? _configFile = result.GetValue(ToolExecuteCommandParser.ConfigOption);
     private readonly string[] _sources = result.GetValue(ToolExecuteCommandParser.SourceOption) ?? [];
     private readonly string[] _addSource = result.GetValue(ToolExecuteCommandParser.AddSourceOption) ?? [];
-    private readonly bool _ignoreFailedSources = result.GetValue(ToolCommandRestorePassThroughOptions.IgnoreFailedSourcesOption);
     private readonly bool _interactive = result.GetValue(ToolExecuteCommandParser.InteractiveOption);
     private readonly VerbosityOptions _verbosity = result.GetValue(ToolExecuteCommandParser.VerbosityOption);
     private readonly bool _yes = result.GetValue(ToolExecuteCommandParser.YesOption);
+    private readonly IToolPackageDownloader _toolPackageDownloader = ToolPackageFactory.CreateToolPackageStoresAndDownloader().downloader;
 
-    //  TODO: Does result.OptionValuesToBeForwarded work here?
-    private readonly IToolPackageDownloader _toolPackageDownloader = ToolPackageFactory.CreateToolPackageStoresAndDownloader(
-            additionalRestoreArguments: result.OptionValuesToBeForwarded(ToolExecuteCommandParser.GetCommand())).downloader;
-
-    //  TODO: Make sure to add these options to the command
     private readonly RestoreActionConfig _restoreActionConfig = new RestoreActionConfig(DisableParallel: result.GetValue(ToolCommandRestorePassThroughOptions.DisableParallelOption),
         NoCache: result.GetValue(ToolCommandRestorePassThroughOptions.NoCacheOption) || result.GetValue(ToolCommandRestorePassThroughOptions.NoHttpCacheOption),
         IgnoreFailedSources: result.GetValue(ToolCommandRestorePassThroughOptions.IgnoreFailedSourcesOption),
-        Interactive: result.GetValue(ToolCommandRestorePassThroughOptions.InteractiveRestoreOption));
+        Interactive: result.GetValue(ToolExecuteCommandParser.InteractiveOption));
 
     private readonly ToolManifestFinder _toolManifestFinder = toolManifestFinder ?? new ToolManifestFinder(new DirectoryPath(currentWorkingDirectory ?? Directory.GetCurrentDirectory()));
 
@@ -92,15 +87,12 @@ public override int Execute()
                 sourceFeedOverrides: _sources,
                 additionalFeeds: _addSource);
 
-        var restoreActionConfig = new RestoreActionConfig(
-                IgnoreFailedSources: _ignoreFailedSources,
-                Interactive: _interactive);
-
-        (var bestVersion, var packageSource) = _toolPackageDownloader.GetNuGetVersion(packageLocation, packageId, _verbosity, versionRange, restoreActionConfig);
+        (var bestVersion, var packageSource) = _toolPackageDownloader.GetNuGetVersion(packageLocation, packageId, _verbosity, versionRange, _restoreActionConfig);
 
         IToolPackage toolPackage;
 
-        //  TODO: Add framework argument
+        //  TargetFramework is null, which means to use the current framework.  Global tools can override the target framework to use (or select assets for),
+        //  but we don't support this for local or one-shot tools.
         if (!_toolPackageDownloader.TryGetDownloadedTool(packageId, bestVersion, targetFramework: null, out toolPackage))
         {
             if (!UserAgreedToRunFromSource(packageId, bestVersion, packageSource))
@@ -132,7 +124,7 @@ public override int Execute()
                 verbosity: _verbosity,
                 versionRange: new VersionRange(bestVersion, true, bestVersion, true),
                 isGlobalToolRollForward: false,
-                restoreActionConfig: restoreActionConfig);
+                restoreActionConfig: _restoreActionConfig);
         }
 
         var commandSpec = ToolCommandSpecCreator.CreateToolCommandSpec(toolPackage.Command.Name.Value, toolPackage.Command.Executable.Value, toolPackage.Command.Runner, _allowRollForward, _forwardArguments);
@@ -153,8 +145,6 @@ private bool UserAgreedToRunFromSource(PackageId packageId, NuGetVersion version
             return false;
         }
 
-        //  TODO: Use a better way to ask for user input
-        //  TODO: How to localize y/n and interpret keys correctly?  Does Spectre.Console handle this?
         string promptMessage = string.Format(CliCommandStrings.ToolDownloadConfirmationPrompt, packageId, version.ToString(), source.Source);
 
         static string AddPromptOptions(string message)

src/Cli/dotnet/Commands/Tool/Execute/ToolExecuteCommandParser.cs

@@ -23,7 +23,6 @@ internal static class ToolExecuteCommandParser
     public static readonly Option<string> ConfigOption = ToolInstallCommandParser.ConfigOption;
     public static readonly Option<string[]> SourceOption = ToolInstallCommandParser.SourceOption;
     public static readonly Option<string[]> AddSourceOption = ToolInstallCommandParser.AddSourceOption;
-    public static readonly Option<bool> IgnoreFailedSourcesOption = ToolCommandRestorePassThroughOptions.IgnoreFailedSourcesOption;
     public static readonly Option<bool> InteractiveOption = CommonOptions.InteractiveOption();
     public static readonly Option<bool> YesOption = CommonOptions.YesOption;
     public static readonly Option<VerbosityOptions> VerbosityOption = ToolInstallCommandParser.VerbosityOption;
@@ -45,18 +44,19 @@ private static Command ConstructCommand()
         command.Arguments.Add(CommandArgument);
 
         command.Options.Add(VersionOption);
+        command.Options.Add(YesOption);
+        command.Options.Add(InteractiveOption);
         command.Options.Add(RollForwardOption);
         command.Options.Add(PrereleaseOption);
         command.Options.Add(ConfigOption);
         command.Options.Add(SourceOption);
         command.Options.Add(AddSourceOption);
-        command.Options.Add(IgnoreFailedSourcesOption);
-        command.Options.Add(InteractiveOption);
-        command.Options.Add(YesOption);
+        command.Options.Add(ToolCommandRestorePassThroughOptions.DisableParallelOption);
+        command.Options.Add(ToolCommandRestorePassThroughOptions.IgnoreFailedSourcesOption);
+        command.Options.Add(ToolCommandRestorePassThroughOptions.NoCacheOption);
+        command.Options.Add(ToolCommandRestorePassThroughOptions.NoHttpCacheOption);
         command.Options.Add(VerbosityOption);
 
-        //  TODO: Framework?
-
         command.SetAction((parseResult) => new ToolExecuteCommand(parseResult).Execute());
 
         return command;

src/Cli/dotnet/Commands/Tool/Install/ToolInstallLocalInstaller.cs

@@ -39,7 +39,7 @@ public ToolInstallLocalInstaller(
             IToolPackageStoreQuery,
             IToolPackageDownloader downloader) toolPackageStoresAndDownloader
                 = ToolPackageFactory.CreateToolPackageStoresAndDownloader(
-                    additionalRestoreArguments: parseResult.OptionValuesToBeForwarded(ToolInstallCommandParser.GetCommand()), runtimeJsonPathForTests: runtimeJsonPathForTests);
+                    runtimeJsonPathForTests: runtimeJsonPathForTests);
         _toolPackageDownloader = toolPackageDownloader ?? toolPackageStoresAndDownloader.downloader;
         _restoreActionConfig = restoreActionConfig;
 

src/Cli/dotnet/Commands/Tool/Restore/ToolRestoreCommand.cs

@@ -43,8 +43,7 @@ public ToolRestoreCommand(
             (IToolPackageStore,
                 IToolPackageStoreQuery,
                 IToolPackageDownloader downloader) toolPackageStoresAndInstaller
-                    = ToolPackageFactory.CreateToolPackageStoresAndDownloader(
-                        additionalRestoreArguments: result.OptionValuesToBeForwarded(ToolRestoreCommandParser.GetCommand()));
+                    = ToolPackageFactory.CreateToolPackageStoresAndDownloader();
             _toolPackageDownloader = toolPackageStoresAndInstaller.downloader;
         }
         else

src/Cli/dotnet/Commands/Tool/Update/ToolUpdateLocalCommand.cs

@@ -38,8 +38,7 @@ public ToolUpdateLocalCommand(
             (IToolPackageStore,
                 IToolPackageStoreQuery,
                 IToolPackageDownloader downloader) toolPackageStoresAndDownloader
-                    = ToolPackageFactory.CreateToolPackageStoresAndDownloader(
-                        additionalRestoreArguments: parseResult.OptionValuesToBeForwarded(ToolUpdateCommandParser.GetCommand()));
+                    = ToolPackageFactory.CreateToolPackageStoresAndDownloader();
             _toolPackageDownloader = toolPackageStoresAndDownloader.downloader;
         }
         else

src/Cli/dotnet/ToolPackage/ToolPackageFactory.cs

@@ -11,7 +11,7 @@ namespace Microsoft.DotNet.Cli.ToolPackage;
 internal static class ToolPackageFactory
 {
     public static (IToolPackageStore packageStore, IToolPackageStoreQuery packageStoreQuery, IToolPackageDownloader downloader) CreateToolPackageStoresAndDownloader(
-        DirectoryPath? nonGlobalLocation = null, IEnumerable<string> additionalRestoreArguments = null, string runtimeJsonPathForTests = null)
+        DirectoryPath? nonGlobalLocation = null, string runtimeJsonPathForTests = null)
     {
         ToolPackageStoreAndQuery toolPackageStore = CreateConcreteToolPackageStore(nonGlobalLocation);
         var toolPackageDownloader = new ToolPackageDownloader(toolPackageStore, runtimeJsonPathForTests);