code_review: PR #2043

love-linger · love-linger · commit 28bed1c3645d · 2026-01-13T11:06:21.000+08:00
- Opening repo as folder in VS should always be available
- Use single `_optionsGenerator` instead of `_execArgsGenerator` and `_subOptionsFinder`
- Make sure path is always quoted

Signed-off-by: leo &lt;longshuang@msn.cn&gt;
diff --git a/src/Models/ExternalTool.cs b/src/Models/ExternalTool.cs
@@ -16,12 +16,12 @@ public class ExternalTool
         public string ExecFile { get; }
         public Bitmap IconImage { get; }
 
-        public ExternalTool(string name, string icon, string execFile, Func<string, string> execArgsGenerator = null, Func<string, List<string>> subOptionsFinder = null)
+        public ExternalTool(string name, string icon, string execFile, Func<string, List<string>> optionsGenerator = null)
         {
             Name = name;
             ExecFile = execFile;
-            _execArgsGenerator = execArgsGenerator ?? (path => path.Quoted());
-            _subOptionsFinder = subOptionsFinder;
+
+            _optionsGenerator = optionsGenerator;
 
             try
             {
@@ -35,30 +35,25 @@ public ExternalTool(string name, string icon, string execFile, Func<string, stri
             }
         }
 
-        public void Open(string path)
+        public List<string> MakeLaunchOptions(string repo)
         {
-            // The executable file may be removed after the tool list is loaded (once time on startup).
-            if (!File.Exists(ExecFile))
-                return;
-
-            Process.Start(new ProcessStartInfo()
-            {
-                FileName = ExecFile,
-                Arguments = _execArgsGenerator.Invoke(path),
-                UseShellExecute = false,
-            });
+            return _optionsGenerator?.Invoke(repo) ?? [repo.Quoted()];
         }
 
-        public List<string> FindSubOptions(string path)
+        public void Launch(string args)
         {
-            if (_subOptionsFinder == null)
-                return null;
-
-            return _subOptionsFinder.Invoke(path);
+            if (File.Exists(ExecFile))
+            {
+                Process.Start(new ProcessStartInfo()
+                {
+                    FileName = ExecFile,
+                    Arguments = args,
+                    UseShellExecute = false,
+                });
+            }
         }
 
-        private Func<string, string> _execArgsGenerator = null;
-        private Func<string, List<string>> _subOptionsFinder = null;
+        private Func<string, List<string>> _optionsGenerator = null;
     }
 
     public class VisualStudioInstance
@@ -140,20 +135,20 @@ public ExternalToolsFinder()
             _customization ??= new ExternalToolCustomization();
         }
 
-        public void TryAdd(string name, string icon, Func<string> finder, Func<string, string> execArgsGenerator = null, Func<string, List<string>> subOptionsFinder = null)
+        public void TryAdd(string name, string icon, Func<string> finder, Func<string, List<string>> optionsGenerator = null)
         {
             if (_customization.Excludes.Contains(name))
                 return;
 
             if (_customization.Tools.TryGetValue(name, out var customPath) && File.Exists(customPath))
             {
-                Tools.Add(new ExternalTool(name, icon, customPath, execArgsGenerator, subOptionsFinder));
+                Tools.Add(new ExternalTool(name, icon, customPath, optionsGenerator));
             }
             else
             {
                 var path = finder();
                 if (!string.IsNullOrEmpty(path) && File.Exists(path))
-                    Tools.Add(new ExternalTool(name, icon, path, execArgsGenerator, subOptionsFinder));
+                    Tools.Add(new ExternalTool(name, icon, path, optionsGenerator));
             }
         }
 
diff --git a/src/Native/Windows.cs b/src/Native/Windows.cs
@@ -401,7 +401,7 @@ private void FindVisualStudio(Models.ExternalToolsFinder finder)
                     {
                         var exec = instance.ProductPath;
                         var icon = instance.IsPrerelease ? "vs-preview" : "vs";
-                        finder.TryAdd(instance.DisplayName, icon, () => exec, GenerateCommandlineArgsForVisualStudio, FindVisualStudioSolutions);
+                        finder.TryAdd(instance.DisplayName, icon, () => exec, GenerateVSProjectLaunchOptions);
                     }
                 }
             }
@@ -444,35 +444,33 @@ private void OpenFolderAndSelectFile(string folderPath)
             }
         }
 
-        private string GenerateCommandlineArgsForVisualStudio(string path)
+        private List<string> GenerateVSProjectLaunchOptions(string path)
         {
-            var solutions = FindVisualStudioSolutions(path);
-            return solutions.Count > 0 ? solutions[0].Quoted() : path.Quoted();
-        }
-
-        public List<string> FindVisualStudioSolutions(string path)
-        {
-            var solutions = new List<string>();
-            if (!Directory.Exists(path))
-                return solutions;
+            var options = new List<string>();
+            options.Add(path.Replace("/", "\\").Quoted());
 
-            void Search(DirectoryInfo dir, int depth)
+            if (Directory.Exists(path))
             {
-                if (depth < 0)
-                    return;
+                void Search(List<string> opts, DirectoryInfo dir, int depth)
+                {
+                    if (depth < 0)
+                        return;
 
-                foreach (var file in dir.GetFiles("*.sln"))
-                    solutions.Add(file.FullName);
+                    foreach (var file in dir.GetFiles())
+                    {
+                        if (file.Name.EndsWith(".sln", StringComparison.OrdinalIgnoreCase) ||
+                            file.Name.EndsWith(".slnx", StringComparison.OrdinalIgnoreCase))
+                            opts.Add(file.FullName.Quoted());
+                    }
 
-                foreach (var file in dir.GetFiles("*.slnx"))
-                    solutions.Add(file.FullName);
+                    foreach (var subDir in dir.GetDirectories())
+                        Search(opts, subDir, depth - 1);
+                }
 
-                foreach (var subDir in dir.GetDirectories())
-                    Search(subDir, depth - 1);
+                Search(options, new DirectoryInfo(path), 4);
             }
-            Search(new DirectoryInfo(path), 4);
 
-            return solutions;
+            return options;
         }
     }
 }
diff --git a/src/ViewModels/CommitDetail.cs b/src/ViewModels/CommitDetail.cs
@@ -343,7 +343,7 @@ await Commands.SaveRevisionFile
             if (tool == null)
                 Native.OS.OpenWithDefaultEditor(tmpFile);
             else
-                tool.Open(tmpFile);
+                tool.Launch(tmpFile.Quoted());
         }
 
         public async Task SaveRevisionFileAsync(Models.Object file, string saveTo)
diff --git a/src/Views/RepositoryToolbar.axaml.cs b/src/Views/RepositoryToolbar.axaml.cs
@@ -1,5 +1,4 @@
 using System.Collections.Generic;
-using System.IO;
 
 using Avalonia.Controls;
 using Avalonia.Input;
@@ -63,16 +62,16 @@ private void OpenWithExternalTools(object sender, RoutedEventArgs ev)
                         item.Header = App.Text("Repository.OpenIn", dupTool.Name);
                         item.Icon = new Image { Width = 16, Height = 16, Source = dupTool.IconImage };
 
-                        var subOptions = dupTool.FindSubOptions(fullpath);
-                        if (subOptions != null && subOptions.Count > 1)
+                        var options = dupTool.MakeLaunchOptions(fullpath);
+                        if (options.Count > 1)
                         {
-                            foreach (var subOption in subOptions)
+                            foreach (var opt in options)
                             {
                                 var subItem = new MenuItem();
-                                subItem.Header = Path.GetFileName(subOption);
+                                subItem.Header = opt.Trim('"');
                                 subItem.Click += (_, e) =>
                                 {
-                                    dupTool.Open(subOption);
+                                    dupTool.Launch(opt);
                                     e.Handled = true;
                                 };
 
@@ -85,7 +84,7 @@ private void OpenWithExternalTools(object sender, RoutedEventArgs ev)
                         {
                             item.Click += (_, e) =>
                             {
-                                dupTool.Open(fullpath);
+                                dupTool.Launch(options[0]);
                                 e.Handled = true;
                             };
 
diff --git a/src/Views/WorkingCopy.axaml.cs b/src/Views/WorkingCopy.axaml.cs
@@ -1290,7 +1290,7 @@ private void TryAddOpenFileToContextMenu(ContextMenu menu, string fullpath)
                         item.Icon = new Image { Width = 16, Height = 16, Source = tool.IconImage };
                         item.Click += (_, e) =>
                         {
-                            tool.Open(fullpath);
+                            tool.Launch(fullpath.Quoted());
                             e.Handled = true;
                         };
 

Original file line number	Diff line number	Diff line change
`@@ -16,12 +16,12 @@ public class ExternalTool`
`16`	`16`	`public string ExecFile { get; }`
`17`	`17`	`public Bitmap IconImage { get; }`
`18`	`18`
`19`		`- public ExternalTool(string name, string icon, string execFile, Func<string, string> execArgsGenerator = null, Func<string, List<string>> subOptionsFinder = null)`
	`19`	`+ public ExternalTool(string name, string icon, string execFile, Func<string, List<string>> optionsGenerator = null)`
`20`	`20`	`{`
`21`	`21`	`Name = name;`
`22`	`22`	`ExecFile = execFile;`
`23`		`- _execArgsGenerator = execArgsGenerator ?? (path => path.Quoted());`
`24`		`- _subOptionsFinder = subOptionsFinder;`
	`23`	`+`
	`24`	`+ _optionsGenerator = optionsGenerator;`
`25`	`25`
`26`	`26`	`try`
`27`	`27`	`{`
`@@ -35,30 +35,25 @@ public ExternalTool(string name, string icon, string execFile, Func<string, stri`
`35`	`35`	`}`
`36`	`36`	`}`
`37`	`37`
`38`		`- public void Open(string path)`
	`38`	`+ public List<string> MakeLaunchOptions(string repo)`
`39`	`39`	`{`
`40`		`- // The executable file may be removed after the tool list is loaded (once time on startup).`
`41`		`- if (!File.Exists(ExecFile))`
`42`		`- return;`
`43`		`-`
`44`		`- Process.Start(new ProcessStartInfo()`
`45`		`- {`
`46`		`- FileName = ExecFile,`
`47`		`- Arguments = _execArgsGenerator.Invoke(path),`
`48`		`- UseShellExecute = false,`
`49`		`- });`
	`40`	`+ return _optionsGenerator?.Invoke(repo) ?? [repo.Quoted()];`
`50`	`41`	`}`
`51`	`42`
`52`		`- public List<string> FindSubOptions(string path)`
	`43`	`+ public void Launch(string args)`
`53`	`44`	`{`
`54`		`- if (_subOptionsFinder == null)`
`55`		`- return null;`
`56`		`-`
`57`		`- return _subOptionsFinder.Invoke(path);`
	`45`	`+ if (File.Exists(ExecFile))`
	`46`	`+ {`
	`47`	`+ Process.Start(new ProcessStartInfo()`
	`48`	`+ {`
	`49`	`+ FileName = ExecFile,`
	`50`	`+ Arguments = args,`
	`51`	`+ UseShellExecute = false,`
	`52`	`+ });`
	`53`	`+ }`
`58`	`54`	`}`
`59`	`55`
`60`		`- private Func<string, string> _execArgsGenerator = null;`
`61`		`- private Func<string, List<string>> _subOptionsFinder = null;`
	`56`	`+ private Func<string, List<string>> _optionsGenerator = null;`
`62`	`57`	`}`
`63`	`58`
`64`	`59`	`public class VisualStudioInstance`
`@@ -140,20 +135,20 @@ public ExternalToolsFinder()`
`140`	`135`	`_customization ??= new ExternalToolCustomization();`
`141`	`136`	`}`
`142`	`137`
`143`		`- public void TryAdd(string name, string icon, Func<string> finder, Func<string, string> execArgsGenerator = null, Func<string, List<string>> subOptionsFinder = null)`
	`138`	`+ public void TryAdd(string name, string icon, Func<string> finder, Func<string, List<string>> optionsGenerator = null)`
`144`	`139`	`{`
`145`	`140`	`if (_customization.Excludes.Contains(name))`
`146`	`141`	`return;`
`147`	`142`
`148`	`143`	`if (_customization.Tools.TryGetValue(name, out var customPath) && File.Exists(customPath))`
`149`	`144`	`{`
`150`		`- Tools.Add(new ExternalTool(name, icon, customPath, execArgsGenerator, subOptionsFinder));`
	`145`	`+ Tools.Add(new ExternalTool(name, icon, customPath, optionsGenerator));`
`151`	`146`	`}`
`152`	`147`	`else`
`153`	`148`	`{`
`154`	`149`	`var path = finder();`
`155`	`150`	`if (!string.IsNullOrEmpty(path) && File.Exists(path))`
`156`		`- Tools.Add(new ExternalTool(name, icon, path, execArgsGenerator, subOptionsFinder));`
	`151`	`+ Tools.Add(new ExternalTool(name, icon, path, optionsGenerator));`
`157`	`152`	`}`
`158`	`153`	`}`
`159`	`154`
Original file line number	Diff line number	Diff line change
`@@ -401,7 +401,7 @@ private void FindVisualStudio(Models.ExternalToolsFinder finder)`
`401`	`401`	`{`
`402`	`402`	`var exec = instance.ProductPath;`
`403`	`403`	`var icon = instance.IsPrerelease ? "vs-preview" : "vs";`
`404`		`- finder.TryAdd(instance.DisplayName, icon, () => exec, GenerateCommandlineArgsForVisualStudio, FindVisualStudioSolutions);`
	`404`	`+ finder.TryAdd(instance.DisplayName, icon, () => exec, GenerateVSProjectLaunchOptions);`
`405`	`405`	`}`
`406`	`406`	`}`
`407`	`407`	`}`
`@@ -444,35 +444,33 @@ private void OpenFolderAndSelectFile(string folderPath)`
`444`	`444`	`}`
`445`	`445`	`}`
`446`	`446`
`447`		`- private string GenerateCommandlineArgsForVisualStudio(string path)`
	`447`	`+ private List<string> GenerateVSProjectLaunchOptions(string path)`
`448`	`448`	`{`
`449`		`- var solutions = FindVisualStudioSolutions(path);`
`450`		`- return solutions.Count > 0 ? solutions[0].Quoted() : path.Quoted();`
`451`		`- }`
`452`		`-`
`453`		`- public List<string> FindVisualStudioSolutions(string path)`
`454`		`- {`
`455`		`- var solutions = new List<string>();`
`456`		`- if (!Directory.Exists(path))`
`457`		`- return solutions;`
	`449`	`+ var options = new List<string>();`
	`450`	`+ options.Add(path.Replace("/", "\\").Quoted());`
`458`	`451`
`459`		`- void Search(DirectoryInfo dir, int depth)`
	`452`	`+ if (Directory.Exists(path))`
`460`	`453`	`{`
`461`		`- if (depth < 0)`
`462`		`- return;`
	`454`	`+ void Search(List<string> opts, DirectoryInfo dir, int depth)`
	`455`	`+ {`
	`456`	`+ if (depth < 0)`
	`457`	`+ return;`
`463`	`458`
`464`		`- foreach (var file in dir.GetFiles("*.sln"))`
`465`		`- solutions.Add(file.FullName);`
	`459`	`+ foreach (var file in dir.GetFiles())`
	`460`	`+ {`
	`461`	`+ if (file.Name.EndsWith(".sln", StringComparison.OrdinalIgnoreCase) \|\|`
	`462`	`+ file.Name.EndsWith(".slnx", StringComparison.OrdinalIgnoreCase))`
	`463`	`+ opts.Add(file.FullName.Quoted());`
	`464`	`+ }`
`466`	`465`
`467`		`- foreach (var file in dir.GetFiles("*.slnx"))`
`468`		`- solutions.Add(file.FullName);`
	`466`	`+ foreach (var subDir in dir.GetDirectories())`
	`467`	`+ Search(opts, subDir, depth - 1);`
	`468`	`+ }`
`469`	`469`
`470`		`- foreach (var subDir in dir.GetDirectories())`
`471`		`- Search(subDir, depth - 1);`
	`470`	`+ Search(options, new DirectoryInfo(path), 4);`
`472`	`471`	`}`
`473`		`- Search(new DirectoryInfo(path), 4);`
`474`	`472`
`475`		`- return solutions;`
	`473`	`+ return options;`
`476`	`474`	`}`
`477`	`475`	`}`
`478`	`476`	`}`
Original file line number	Diff line number	Diff line change
`@@ -343,7 +343,7 @@ await Commands.SaveRevisionFile`
`343`	`343`	`if (tool == null)`
`344`	`344`	`Native.OS.OpenWithDefaultEditor(tmpFile);`
`345`	`345`	`else`
`346`		`- tool.Open(tmpFile);`
	`346`	`+ tool.Launch(tmpFile.Quoted());`
`347`	`347`	`}`
`348`	`348`
`349`	`349`	`public async Task SaveRevisionFileAsync(Models.Object file, string saveTo)`
Original file line number	Diff line number	Diff line change
`@@ -1290,7 +1290,7 @@ private void TryAddOpenFileToContextMenu(ContextMenu menu, string fullpath)`
`1290`	`1290`	`item.Icon = new Image { Width = 16, Height = 16, Source = tool.IconImage };`
`1291`	`1291`	`item.Click += (_, e) =>`
`1292`	`1292`	`{`
`1293`		`- tool.Open(fullpath);`
	`1293`	`+ tool.Launch(fullpath.Quoted());`
`1294`	`1294`	`e.Handled = true;`
`1295`	`1295`	`};`
`1296`	`1296`