code_review: PR #1525

love-linger · love-linger · commit eb45380b8da4 · 2025-07-05T16:22:52.000+08:00
- `OpenForDiffAsync` should not use `await` (we do not need to wait it ends)
- Modify `Checkout`, `CheckoutCommit` and `CreateBranch`

Signed-off-by: leo &lt;longshuang@msn.cn&gt;
diff --git a/src/App.axaml.cs b/src/App.axaml.cs
@@ -186,7 +186,9 @@ public static void SendNotification(string context, string message)
 
         public static void SetLocale(string localeKey)
         {
-            if (Current is not App app || app.Resources[localeKey] is not ResourceDictionary targetLocale || targetLocale == app._activeLocale)
+            if (Current is not App app ||
+                app.Resources[localeKey] is not ResourceDictionary targetLocale ||
+                targetLocale == app._activeLocale)
                 return;
 
             if (app._activeLocale != null)
diff --git a/src/ViewModels/BranchCompare.cs b/src/ViewModels/BranchCompare.cs
@@ -125,13 +125,13 @@ public ContextMenu CreateChangeContextMenu()
             var diffWithMerger = new MenuItem();
             diffWithMerger.Header = App.Text("DiffWithMerger");
             diffWithMerger.Icon = App.CreateMenuIcon("Icons.OpenWith");
-            diffWithMerger.Click += async (_, ev) =>
+            diffWithMerger.Click += (sender, ev) =>
             {
                 var toolType = Preferences.Instance.ExternalMergeToolType;
                 var toolPath = Preferences.Instance.ExternalMergeToolPath;
                 var opt = new Models.DiffOption(_based.Head, _to.Head, change);
 
-                await Commands.MergeTool.OpenForDiffAsync(_repo, toolType, toolPath, opt);
+                _ = Commands.MergeTool.OpenForDiffAsync(_repo, toolType, toolPath, opt);
                 ev.Handled = true;
             };
             menu.Items.Add(diffWithMerger);
diff --git a/src/ViewModels/Checkout.cs b/src/ViewModels/Checkout.cs
@@ -58,14 +58,8 @@ public override async Task<bool> Sure()
 
             var succ = false;
             var needPopStash = false;
-
-            if (DiscardLocalChanges)
-            {
-                succ = await new Commands.Checkout(_repo.FullPath)
-                    .Use(log)
-                    .BranchAsync(Branch, true);
-            }
-            else
+            
+            if (!DiscardLocalChanges)
             {
                 var changes = await new Commands.CountLocalChangesWithoutUntracked(_repo.FullPath).GetResultAsync();
                 if (changes > 0)
@@ -82,11 +76,11 @@ public override async Task<bool> Sure()
 
                     needPopStash = true;
                 }
-
-                succ = await new Commands.Checkout(_repo.FullPath)
-                    .Use(log)
-                    .BranchAsync(Branch, false);
             }
+            
+            succ = await new Commands.Checkout(_repo.FullPath)
+                .Use(log)
+                .BranchAsync(Branch, DiscardLocalChanges);
 
             if (succ)
             {
diff --git a/src/ViewModels/CheckoutAndFastForward.cs b/src/ViewModels/CheckoutAndFastForward.cs
@@ -92,7 +92,6 @@ public override async Task<bool> Sure()
                 if (IsRecurseSubmoduleVisible && RecurseSubmodules)
                 {
                     var submodules = await new Commands.QueryUpdatableSubmodules(_repo.FullPath).GetResultAsync();
-
                     if (submodules.Count > 0)
                         await new Commands.Submodule(_repo.FullPath)
                             .Use(log)
diff --git a/src/ViewModels/CheckoutCommit.cs b/src/ViewModels/CheckoutCommit.cs
@@ -41,10 +41,6 @@ public override async Task<bool> Sure()
             var log = _repo.CreateLog("Checkout Commit");
             Use(log);
 
-            var updateSubmodules = IsRecurseSubmoduleVisible && RecurseSubmodules;
-            bool succ;
-            var needPop = false;
-
             if (_repo.CurrentBranch is { IsDetachedHead: true })
             {
                 var refs = await new Commands.QueryRefsContainsCommit(_repo.FullPath, _repo.CurrentBranch.Head).GetResultAsync();
@@ -59,14 +55,11 @@ public override async Task<bool> Sure()
                     }
                 }
             }
+            
+            var succ = false;
+            var needPop = false;
 
-            if (DiscardLocalChanges)
-            {
-                succ = await new Commands.Checkout(_repo.FullPath)
-                    .Use(log)
-                    .CommitAsync(Commit.SHA, true);
-            }
-            else
+            if (!DiscardLocalChanges)
             {
                 var changes = await new Commands.CountLocalChangesWithoutUntracked(_repo.FullPath).GetResultAsync();
                 if (changes > 0)
@@ -83,15 +76,15 @@ public override async Task<bool> Sure()
 
                     needPop = true;
                 }
-
-                succ = await new Commands.Checkout(_repo.FullPath)
-                    .Use(log)
-                    .CommitAsync(Commit.SHA, false);
             }
+            
+            succ = await new Commands.Checkout(_repo.FullPath)
+                .Use(log)
+                .CommitAsync(Commit.SHA, DiscardLocalChanges);
 
             if (succ)
             {
-                if (updateSubmodules)
+                if (IsRecurseSubmoduleVisible && RecurseSubmodules)
                 {
                     var submodules = await new Commands.QueryUpdatableSubmodules(_repo.FullPath).GetResultAsync();
                     if (submodules.Count > 0)
diff --git a/src/ViewModels/CommitDetail.cs b/src/ViewModels/CommitDetail.cs
@@ -325,13 +325,13 @@ public ContextMenu CreateChangeContextMenu(Models.Change change)
             var diffWithMerger = new MenuItem();
             diffWithMerger.Header = App.Text("DiffWithMerger");
             diffWithMerger.Icon = App.CreateMenuIcon("Icons.OpenWith");
-            diffWithMerger.Click += async (_, ev) =>
+            diffWithMerger.Click += (sender, ev) =>
             {
                 var toolType = Preferences.Instance.ExternalMergeToolType;
                 var toolPath = Preferences.Instance.ExternalMergeToolPath;
                 var opt = new Models.DiffOption(_commit, change);
 
-                await Commands.MergeTool.OpenForDiffAsync(_repo.FullPath, toolType, toolPath, opt);
+                _ = Commands.MergeTool.OpenForDiffAsync(_repo.FullPath, toolType, toolPath, opt);
                 ev.Handled = true;
             };
 
diff --git a/src/ViewModels/CreateBranch.cs b/src/ViewModels/CreateBranch.cs
@@ -123,9 +123,6 @@ public override async Task<bool> Sure()
             var log = _repo.CreateLog($"Create Branch '{fixedName}'");
             Use(log);
 
-            var updateSubmodules = IsRecurseSubmoduleVisible && RecurseSubmodules;
-            bool succ;
-
             if (CheckoutAfterCreated)
             {
                 if (_repo.CurrentBranch is { IsDetachedHead: true } && !_repo.CurrentBranch.Head.Equals(_baseOnRevision, StringComparison.Ordinal))
@@ -143,17 +140,12 @@ public override async Task<bool> Sure()
                     }
                 }
             }
-
+            
+            bool succ;
             if (CheckoutAfterCreated && !_repo.IsBare)
             {
                 var needPopStash = false;
-                if (DiscardLocalChanges)
-                {
-                    succ = await new Commands.Checkout(_repo.FullPath)
-                        .Use(log)
-                        .BranchAsync(fixedName, _baseOnRevision, true, _allowOverwrite);
-                }
-                else
+                if (!DiscardLocalChanges)
                 {
                     var changes = await new Commands.CountLocalChangesWithoutUntracked(_repo.FullPath).GetResultAsync();
                     if (changes > 0)
@@ -170,15 +162,15 @@ public override async Task<bool> Sure()
 
                         needPopStash = true;
                     }
-
-                    succ = await new Commands.Checkout(_repo.FullPath)
-                        .Use(log)
-                        .BranchAsync(fixedName, _baseOnRevision, false, _allowOverwrite);
                 }
+                
+                succ = await new Commands.Checkout(_repo.FullPath)
+                    .Use(log)
+                    .BranchAsync(fixedName, _baseOnRevision, DiscardLocalChanges, _allowOverwrite);
 
                 if (succ)
                 {
-                    if (updateSubmodules)
+                    if (IsRecurseSubmoduleVisible && RecurseSubmodules)
                     {
                         var submodules = await new Commands.QueryUpdatableSubmodules(_repo.FullPath).GetResultAsync();
                         if (submodules.Count > 0)
diff --git a/src/ViewModels/Histories.cs b/src/ViewModels/Histories.cs
@@ -599,7 +599,11 @@ public ContextMenu MakeContextMenu(DataGrid list)
                                 var parents = new List<Models.Commit>();
                                 foreach (var sha in commit.Parents)
                                 {
-                                    var parent = _commits.Find(x => x.SHA == sha) ?? await new Commands.QuerySingleCommit(_repo.FullPath, sha).GetResultAsync();
+                                    var parent = _commits.Find(x => x.SHA == sha);
+                                    if (parent == null)
+                                        parent = await new Commands.QuerySingleCommit(_repo.FullPath, sha)
+                                            .GetResultAsync();
+
                                     if (parent != null)
                                         parents.Add(parent);
                                 }

Original file line number	Diff line number	Diff line change
`@@ -186,7 +186,9 @@ public static void SendNotification(string context, string message)`
`186`	`186`
`187`	`187`	`public static void SetLocale(string localeKey)`
`188`	`188`	`{`
`189`		`- if (Current is not App app \|\| app.Resources[localeKey] is not ResourceDictionary targetLocale \|\| targetLocale == app._activeLocale)`
	`189`	`+ if (Current is not App app \|\|`
	`190`	`+ app.Resources[localeKey] is not ResourceDictionary targetLocale \|\|`
	`191`	`+ targetLocale == app._activeLocale)`
`190`	`192`	`return;`
`191`	`193`
`192`	`194`	`if (app._activeLocale != null)`
Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,6 @@ public override async Task<bool> Sure()`
`92`	`92`	`if (IsRecurseSubmoduleVisible && RecurseSubmodules)`
`93`	`93`	`{`
`94`	`94`	`var submodules = await new Commands.QueryUpdatableSubmodules(_repo.FullPath).GetResultAsync();`
`95`		`-`
`96`	`95`	`if (submodules.Count > 0)`
`97`	`96`	`await new Commands.Submodule(_repo.FullPath)`
`98`	`97`	`.Use(log)`
Original file line number	Diff line number	Diff line change
`@@ -41,10 +41,6 @@ public override async Task<bool> Sure()`
`41`	`41`	`var log = _repo.CreateLog("Checkout Commit");`
`42`	`42`	`Use(log);`
`43`	`43`
`44`		`- var updateSubmodules = IsRecurseSubmoduleVisible && RecurseSubmodules;`
`45`		`- bool succ;`
`46`		`- var needPop = false;`
`47`		`-`
`48`	`44`	`if (_repo.CurrentBranch is { IsDetachedHead: true })`
`49`	`45`	`{`
`50`	`46`	`var refs = await new Commands.QueryRefsContainsCommit(_repo.FullPath, _repo.CurrentBranch.Head).GetResultAsync();`
`@@ -59,14 +55,11 @@ public override async Task<bool> Sure()`
`59`	`55`	`}`
`60`	`56`	`}`
`61`	`57`	`}`
	`58`	`+`
	`59`	`+ var succ = false;`
	`60`	`+ var needPop = false;`
`62`	`61`
`63`		`- if (DiscardLocalChanges)`
`64`		`- {`
`65`		`- succ = await new Commands.Checkout(_repo.FullPath)`
`66`		`- .Use(log)`
`67`		`- .CommitAsync(Commit.SHA, true);`
`68`		`- }`
`69`		`- else`
	`62`	`+ if (!DiscardLocalChanges)`
`70`	`63`	`{`
`71`	`64`	`var changes = await new Commands.CountLocalChangesWithoutUntracked(_repo.FullPath).GetResultAsync();`
`72`	`65`	`if (changes > 0)`
`@@ -83,15 +76,15 @@ public override async Task<bool> Sure()`
`83`	`76`
`84`	`77`	`needPop = true;`
`85`	`78`	`}`
`86`		`-`
`87`		`- succ = await new Commands.Checkout(_repo.FullPath)`
`88`		`- .Use(log)`
`89`		`- .CommitAsync(Commit.SHA, false);`
`90`	`79`	`}`
	`80`	`+`
	`81`	`+ succ = await new Commands.Checkout(_repo.FullPath)`
	`82`	`+ .Use(log)`
	`83`	`+ .CommitAsync(Commit.SHA, DiscardLocalChanges);`
`91`	`84`
`92`	`85`	`if (succ)`
`93`	`86`	`{`
`94`		`- if (updateSubmodules)`
	`87`	`+ if (IsRecurseSubmoduleVisible && RecurseSubmodules)`
`95`	`88`	`{`
`96`	`89`	`var submodules = await new Commands.QueryUpdatableSubmodules(_repo.FullPath).GetResultAsync();`
`97`	`90`	`if (submodules.Count > 0)`
Original file line number	Diff line number	Diff line change
`@@ -123,9 +123,6 @@ public override async Task<bool> Sure()`
`123`	`123`	`var log = _repo.CreateLog($"Create Branch '{fixedName}'");`
`124`	`124`	`Use(log);`
`125`	`125`
`126`		`- var updateSubmodules = IsRecurseSubmoduleVisible && RecurseSubmodules;`
`127`		`- bool succ;`
`128`		`-`
`129`	`126`	`if (CheckoutAfterCreated)`
`130`	`127`	`{`
`131`	`128`	`if (_repo.CurrentBranch is { IsDetachedHead: true } && !_repo.CurrentBranch.Head.Equals(_baseOnRevision, StringComparison.Ordinal))`
`@@ -143,17 +140,12 @@ public override async Task<bool> Sure()`
`143`	`140`	`}`
`144`	`141`	`}`
`145`	`142`	`}`
`146`		`-`
	`143`	`+`
	`144`	`+ bool succ;`
`147`	`145`	`if (CheckoutAfterCreated && !_repo.IsBare)`
`148`	`146`	`{`
`149`	`147`	`var needPopStash = false;`
`150`		`- if (DiscardLocalChanges)`
`151`		`- {`
`152`		`- succ = await new Commands.Checkout(_repo.FullPath)`
`153`		`- .Use(log)`
`154`		`- .BranchAsync(fixedName, _baseOnRevision, true, _allowOverwrite);`
`155`		`- }`
`156`		`- else`
	`148`	`+ if (!DiscardLocalChanges)`
`157`	`149`	`{`
`158`	`150`	`var changes = await new Commands.CountLocalChangesWithoutUntracked(_repo.FullPath).GetResultAsync();`
`159`	`151`	`if (changes > 0)`
`@@ -170,15 +162,15 @@ public override async Task<bool> Sure()`
`170`	`162`
`171`	`163`	`needPopStash = true;`
`172`	`164`	`}`
`173`		`-`
`174`		`- succ = await new Commands.Checkout(_repo.FullPath)`
`175`		`- .Use(log)`
`176`		`- .BranchAsync(fixedName, _baseOnRevision, false, _allowOverwrite);`
`177`	`165`	`}`
	`166`	`+`
	`167`	`+ succ = await new Commands.Checkout(_repo.FullPath)`
	`168`	`+ .Use(log)`
	`169`	`+ .BranchAsync(fixedName, _baseOnRevision, DiscardLocalChanges, _allowOverwrite);`
`178`	`170`
`179`	`171`	`if (succ)`
`180`	`172`	`{`
`181`		`- if (updateSubmodules)`
	`173`	`+ if (IsRecurseSubmoduleVisible && RecurseSubmodules)`
`182`	`174`	`{`
`183`	`175`	`var submodules = await new Commands.QueryUpdatableSubmodules(_repo.FullPath).GetResultAsync();`
`184`	`176`	`if (submodules.Count > 0)`
Original file line number	Diff line number	Diff line change
`@@ -599,7 +599,11 @@ public ContextMenu MakeContextMenu(DataGrid list)`
`599`	`599`	`var parents = new List<Models.Commit>();`
`600`	`600`	`foreach (var sha in commit.Parents)`
`601`	`601`	`{`
`602`		`- var parent = _commits.Find(x => x.SHA == sha) ?? await new Commands.QuerySingleCommit(_repo.FullPath, sha).GetResultAsync();`
	`602`	`+ var parent = _commits.Find(x => x.SHA == sha);`
	`603`	`+ if (parent == null)`
	`604`	`+ parent = await new Commands.QuerySingleCommit(_repo.FullPath, sha)`
	`605`	`+ .GetResultAsync();`
	`606`	`+`
`603`	`607`	`if (parent != null)`
`604`	`608`	`parents.Add(parent);`
`605`	`609`	`}`