code_review: PR #1416

- Split `DoubleTapped` into two methods: `CheckoutBranchByDecorator` and `CheckoutBranchByCommit`
- Move `DoubleTappedEvent` from whole ListBox to the row tapped actually
- Do nothing if the decorator double-clicked is HEAD
- Code-style

Signed-off-by: leo <longshuang@msn.cn>

src/Models/Watcher.cs

@@ -55,7 +55,7 @@ namespace SourceGit.Models
 
                 _watchers.Add(wc);
                 _watchers.Add(git);
-            }               
+            }
 
             _timer = new Timer(Tick, null, 100, 100);
         }

src/ViewModels/Histories.cs

@@ -209,48 +209,56 @@ namespace SourceGit.ViewModels
             }
         }
 
-        public void DoubleTapped(Models.Commit commit, Models.Decorator decorator = null)
+        public bool CheckoutBranchByDecorator(Models.Decorator decorator)
         {
-            if (decorator != null)
+            if (decorator == null)
+                return false;
+
+            if (decorator.Type == Models.DecoratorType.CurrentBranchHead ||
+                decorator.Type == Models.DecoratorType.CurrentCommitHead)
+                return true;
+
+            if (decorator.Type == Models.DecoratorType.LocalBranchHead)
             {
-                if (decorator.Type == Models.DecoratorType.LocalBranchHead)
+                var b = _repo.Branches.Find(x => x.Name == decorator.Name);
-                {
+                if (b == null)
-                    var b = _repo.Branches.Find(x => x.FriendlyName == decorator.Name);
+                    return false;
-                    if (b != null)
+
-                    {
+                _repo.CheckoutBranch(b);
-                        _repo.CheckoutBranch(b);
+                return true;
-                        return;
-                    }
-                }
-                else if (decorator.Type == Models.DecoratorType.RemoteBranchHead)
-                {
-                    var remoteBranch = _repo.Branches.Find(x => x.FriendlyName == decorator.Name);
-                    if (remoteBranch != null)
-                    {
-                        var localBranch = _repo.Branches.Find(x => x.IsLocal && x.Upstream == remoteBranch.FullName);
-                        if (localBranch != null)
-                        {
-                            if (localBranch.IsCurrent)
-                                return;
-                            if (localBranch.TrackStatus.Ahead.Count > 0)
-                            {
-                                if (_repo.CanCreatePopup())
-                                    _repo.ShowPopup(new CreateBranch(_repo, remoteBranch));
-                            }
-                            else if (localBranch.TrackStatus.Behind.Count > 0)
-                            {
-                                if (_repo.CanCreatePopup())
-                                    _repo.ShowPopup(new CheckoutAndFastForward(_repo, localBranch, remoteBranch));
-                            }
-                            else
-                                _repo.CheckoutBranch(localBranch);
-                            return;
-                        }
-                    }
-                }
             }
 
-            if (commit == null || commit.IsCurrentHead)
+            if (decorator.Type == Models.DecoratorType.RemoteBranchHead)
+            {
+                var rb = _repo.Branches.Find(x => x.FriendlyName == decorator.Name);
+                if (rb == null)
+                    return false;
+
+                var lb = _repo.Branches.Find(x => x.IsLocal && x.Upstream == rb.FullName);
+                if (lb == null || lb.TrackStatus.Ahead.Count > 0)
+                {
+                    if (_repo.CanCreatePopup())
+                        _repo.ShowPopup(new CreateBranch(_repo, rb));
+                }
+                else if (lb.TrackStatus.Behind.Count > 0)
+                {
+                    if (_repo.CanCreatePopup())
+                        _repo.ShowPopup(new CheckoutAndFastForward(_repo, lb, rb));
+                }
+                else if (!lb.IsCurrent)
+                {
+                    _repo.CheckoutBranch(lb);
+                }
+
+                return true;
+            }
+
+            return false;
+        }
+
+        public void CheckoutBranchByCommit(Models.Commit commit)
+        {
+            if (commit.IsCurrentHead)
                 return;
 
             var firstRemoteBranch = null as Models.Branch;
@@ -258,28 +266,28 @@ namespace SourceGit.ViewModels
             {
                 if (d.Type == Models.DecoratorType.LocalBranchHead)
                 {
-                    var b = _repo.Branches.Find(x => x.FriendlyName == d.Name);
+                    var b = _repo.Branches.Find(x => x.Name == d.Name);
-                    if (b != null)
+                    if (b == null)
-                    {
+                        continue;
-                        _repo.CheckoutBranch(b);
+
-                        return;
+                    _repo.CheckoutBranch(b);
-                    }
+                    return;
                 }
                 else if (d.Type == Models.DecoratorType.RemoteBranchHead)
                 {
-                    var remoteBranch = _repo.Branches.Find(x => x.FriendlyName == d.Name);
+                    var rb = _repo.Branches.Find(x => x.FriendlyName == d.Name);
-                    if (remoteBranch != null)
+                    if (rb == null)
+                        continue;
+
+                    var lb = _repo.Branches.Find(x => x.IsLocal && x.Upstream == rb.FullName);
+                    if (lb is { TrackStatus.Ahead.Count: 0 })
                     {
-                        var localBranch = _repo.Branches.Find(x => x.IsLocal && x.Upstream == remoteBranch.FullName);
+                        if (_repo.CanCreatePopup())
-                        if (localBranch is { TrackStatus.Ahead.Count: 0 })
+                            _repo.ShowPopup(new CheckoutAndFastForward(_repo, lb, rb));
-                        {
+                        return;
-                            if (_repo.CanCreatePopup())
-                                _repo.ShowPopup(new CheckoutAndFastForward(_repo, localBranch, remoteBranch));
-                            return;
-                        }
                     }
 
-                    firstRemoteBranch ??= remoteBranch;
+                    firstRemoteBranch ??= rb;
                 }
             }
 

src/ViewModels/WorkingCopy.cs

@@ -810,7 +810,7 @@ namespace SourceGit.ViewModels
                                 };
                                 addToIgnore.Items.Add(byExtensionInSameFolder);
                             }
-                        }                       
+                        }
 
                         menu.Items.Add(addToIgnore);
                         hasExtra = true;
@@ -1414,7 +1414,7 @@ namespace SourceGit.ViewModels
 
                     menu.Items.Add(explore);
                     menu.Items.Add(new MenuItem() { Header = "-" });
-                }                
+                }
 
                 var unstage = new MenuItem();
                 unstage.Header = App.Text("FileCM.UnstageMulti", _selectedStaged.Count);

src/Views/CommitRefsPresenter.cs

@@ -17,6 +17,7 @@ namespace SourceGit.Views
             public IBrush Brush { get; set; } = null;
             public bool IsHead { get; set; } = false;
             public double Width { get; set; } = 0.0;
+            public Models.Decorator Decorator { get; set; } = null;
         }
 
         public static readonly StyledProperty<FontFamily> FontFamilyProperty =
@@ -93,6 +94,19 @@ namespace SourceGit.Views
                 ShowTagsProperty);
         }
 
+        public Models.Decorator DecoratorAt(Point point)
+        {
+            var x = 0.0;
+            foreach (var item in _items)
+            {
+                x += item.Width;
+                if (point.X < x)
+                    return item.Decorator;
+            }
+
+            return null;
+        }
+
         public override void Render(DrawingContext context)
         {
             if (_items.Count == 0)
@@ -156,22 +170,6 @@ namespace SourceGit.Views
             InvalidateMeasure();
         }
 
-        public Models.Decorator DecoratorAt(Point point)
-        {
-            if (DataContext is not Models.Commit commit)
-                return null;
-            
-            var x = 0.0;
-            for (var i = 0; i < _items.Count; i++)
-            {
-                x += _items[i].Width + 4;
-                if (point.X < x)
-                    return commit.Decorators[i];
-            }
-            
-            return null;
-        }
-        
         protected override Size MeasureOverride(Size availableSize)
         {
             _items.Clear();
@@ -214,7 +212,8 @@ namespace SourceGit.Views
                     {
                         Label = label,
                         Brush = normalBG,
-                        IsHead = isHead
+                        IsHead = isHead,
+                        Decorator = decorator,
                     };
 
                     StreamGeometry geo;

src/Views/Histories.axaml

@@ -70,7 +70,6 @@
                    LayoutUpdated="OnCommitListLayoutUpdated"
                    SelectionChanged="OnCommitListSelectionChanged"
                    ContextRequested="OnCommitListContextRequested"
-                   DoubleTapped="OnCommitListDoubleTapped"
                    KeyDown="OnCommitListKeyDown">
             <ListBox.Styles>
               <Style Selector="ListBoxItem">
@@ -117,7 +116,7 @@
 
             <ListBox.ItemTemplate>
               <DataTemplate DataType="m:Commit">
-                <Grid Height="26">
+                <Grid Height="26" Background="Transparent" DoubleTapped="OnCommitListItemDoubleTapped">
                   <Grid.ColumnDefinitions>
                     <ColumnDefinition Width="*"/>
                     <ColumnDefinition SharedSizeGroup="AuthorName" Width="Auto"/>

src/Views/Histories.axaml.cs

@@ -169,22 +169,6 @@ namespace SourceGit.Views
             e.Handled = true;
         }
 
-        private void OnCommitListDoubleTapped(object sender, TappedEventArgs e)
-        {
-            if (DataContext is ViewModels.Histories histories && sender is ListBox { SelectedItems.Count: 1 })
-            {
-                Models.Decorator decorator = null;
-                if (e.Source is CommitRefsPresenter crp)
-                    decorator = crp.DecoratorAt(e.GetPosition(crp));
-
-                var source = e.Source as Control;
-                var item = source.FindAncestorOfType<ListBoxItem>();
-                if (item is { DataContext: Models.Commit commit })
-                    histories.DoubleTapped(commit, decorator);
-            }
-            e.Handled = true;
-        }
-
         private void OnCommitListKeyDown(object sender, KeyEventArgs e)
         {
             if (!e.KeyModifiers.HasFlag(OperatingSystem.IsMacOS() ? KeyModifiers.Meta : KeyModifiers.Control))
@@ -228,6 +212,25 @@ namespace SourceGit.Views
             }
         }
 
+        private void OnCommitListItemDoubleTapped(object sender, TappedEventArgs e)
+        {
+            e.Handled = true;
+
+            if (DataContext is ViewModels.Histories histories &&
+                CommitListContainer.SelectedItems is { Count: 1 } &&
+                sender is Grid { DataContext: Models.Commit commit })
+            {
+                if (e.Source is CommitRefsPresenter crp)
+                {
+                    var decorator = crp.DecoratorAt(e.GetPosition(crp));
+                    if (histories.CheckoutBranchByDecorator(decorator))
+                        return;
+                }
+
+                histories.CheckoutBranchByCommit(commit);
+            }
+        }
+
         private double _lastScrollY = 0;
         private double _lastAuthorNameColumnWidth = 0;
     }