code_review: PR #1402

- it's unnecessary to implement `IEnumerable` interface
- we should check `IsIntersecting` before creating `InlineElement` to avoid unnecessary works suck as running `git cat-file -t <hash>`
- sort whold list after all elements have been added to avoid unnecessary memmove in `Insert`

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

src/Models/Commit.cs

@@ -33,8 +33,8 @@ namespace SourceGit.Models
         public User Committer { get; set; } = User.Invalid;
         public ulong CommitterTime { get; set; } = 0;
         public string Subject { get; set; } = string.Empty;
-        public List<string> Parents { get; set; } = new List<string>();
+        public List<string> Parents { get; set; } = new();
-        public List<Decorator> Decorators { get; set; } = new List<Decorator>();
+        public List<Decorator> Decorators { get; set; } = new();
         public bool HasDecorators => Decorators.Count > 0;
 
         public string AuthorTimeStr => DateTime.UnixEpoch.AddSeconds(AuthorTime).ToLocalTime().ToString(DateTimeFormat.Active.DateTime);
@@ -49,7 +49,7 @@ namespace SourceGit.Models
         public int Color { get; set; } = 0;
         public double Opacity => IsMerged ? 1 : OpacityForNotMerged;
         public FontWeight FontWeight => IsCurrentHead ? FontWeight.Bold : FontWeight.Regular;
-        public Thickness Margin { get; set; } = new Thickness(0);
+        public Thickness Margin { get; set; } = new(0);
         public IBrush Brush => CommitGraph.Pens[Color].Brush;
 
         public void ParseDecorators(string data)
@@ -121,6 +121,6 @@ namespace SourceGit.Models
     public class CommitFullMessage
     {
         public string Message { get; set; } = string.Empty;
-        public InlineElementCollector Inlines { get; set; } = [];
+        public InlineElementCollector Inlines { get; set; } = new();
     }
 }

src/Models/InlineElement.cs

@@ -2,8 +2,7 @@
 {
     public enum InlineElementType
     {
-        None = 0,
+        Keyword = 0,
-        Keyword,
         Link,
         CommitSHA,
         Code,
@@ -11,10 +10,10 @@
 
     public class InlineElement
     {
-        public InlineElementType Type { get; set; } = InlineElementType.None;
+        public InlineElementType Type { get; }
-        public int Start { get; set; } = 0;
+        public int Start { get; }
-        public int Length { get; set; } = 0;
+        public int Length { get; }
-        public string Link { get; set; } = "";
+        public string Link { get; }
 
         public InlineElement(InlineElementType type, int start, int length, string link)
         {
@@ -24,7 +23,7 @@
             Link = link;
         }
 
-        public bool Intersect(int start, int length)
+        public bool IsIntersecting(int start, int length)
         {
             if (start == Start)
                 return true;

src/Models/InlineElementCollector.cs

@@ -1,85 +1,38 @@
-using System.Collections;
 using System.Collections.Generic;
-using System.Diagnostics;
 
 namespace SourceGit.Models
 {
-    public class InlineElementCollector : IEnumerable<InlineElement>
+    public class InlineElementCollector
     {
-        private readonly List<InlineElement> _implementation = [];
+        public int Count => _implementation.Count;
+        public InlineElement this[int index] => _implementation[index];
+
+        public InlineElement Intersect(int start, int length)
+        {
+            foreach (var elem in _implementation)
+            {
+                if (elem.IsIntersecting(start, length))
+                    return elem;
+            }
+
+            return null;
+        }
+
+        public void Add(InlineElement element)
+        {
+            _implementation.Add(element);
+        }
+
+        public void Sort()
+        {
+            _implementation.Sort((l, r) => l.Start.CompareTo(r.Start));
+        }
 
         public void Clear()
         {
             _implementation.Clear();
-
-            AssertInvariant();
         }
 
-        public int Count => _implementation.Count;
+        private readonly List<InlineElement> _implementation = [];
-
-        public void Add(InlineElement element)
-        {
-
-            var index = FindIndex(element.Start);
-            if (!IsIntersection(index, element.Start, element.Length))
-                _implementation.Insert(index, element);
-
-            AssertInvariant();
-        }
-
-        [Conditional("DEBUG")]
-        private void AssertInvariant()
-        {
-            if (_implementation.Count == 0)
-                return;
-
-            for (var index = 1; index < _implementation.Count; index++)
-            {
-                var prev = _implementation[index - 1];
-                var curr = _implementation[index];
-
-                Debug.Assert(prev.Start + prev.Length <= curr.Start);
-            }
-        }
-
-        public InlineElement Lookup(int position)
-        {
-            var index = FindIndex(position);
-            return IsIntersection(index, position, 1)
-                ? _implementation[index]
-                : null;
-        }
-
-        private int FindIndex(int start)
-        {
-            var index = 0;
-            while (index < _implementation.Count && _implementation[index].Start <= start)
-                index++;
-
-            return index;
-        }
-
-        private bool IsIntersection(int index, int start, int length)
-        {
-            if (index > 0)
-            {
-                var predecessor = _implementation[index - 1];
-                if (predecessor.Start + predecessor.Length >= start)
-                    return true;
-            }
-
-            if (index < _implementation.Count)
-            {
-                var successor = _implementation[index];
-                if (start + length >= successor.Start)
-                    return true;
-            }
-
-            return false;
-        }
-
-        public IEnumerator<InlineElement> GetEnumerator() => _implementation.GetEnumerator();
-
-        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
     }
 }

src/Models/IssueTrackerRule.cs

@@ -59,6 +59,8 @@ namespace SourceGit.Models
 
                 var start = match.Index;
                 var len = match.Length;
+                if (outs.Intersect(start, len) != null)
+                    continue;
 
                 var link = _urlTemplate;
                 for (var j = 1; j < match.Groups.Count; j++)

src/ViewModels/CommitDetail.cs

@@ -638,6 +638,8 @@ namespace SourceGit.ViewModels
 
                 var start = match.Index;
                 var len = match.Length;
+                if (inlines.Intersect(start, len) != null)
+                    continue;
 
                 var url = message.Substring(start, len);
                 if (Uri.IsWellFormedUriString(url, UriKind.Absolute))
@@ -653,6 +655,8 @@ namespace SourceGit.ViewModels
 
                 var start = match.Index;
                 var len = match.Length;
+                if (inlines.Intersect(start, len) != null)
+                    continue;
 
                 var sha = match.Groups[1].Value;
                 var isCommitSHA = new Commands.IsCommitSHA(_repo.FullPath, sha).Result();
@@ -660,6 +664,7 @@ namespace SourceGit.ViewModels
                     inlines.Add(new Models.InlineElement(Models.InlineElementType.CommitSHA, start, len, sha));
             }
 
+            inlines.Sort();
             return inlines;
         }
 

src/Views/CommitMessagePresenter.cs

@@ -48,8 +48,9 @@ namespace SourceGit.Views
 
                 var inlines = new List<Inline>();
                 var pos = 0;
-                foreach (var link in links)
+                for (var i = 0; i < links.Count; i++)
                 {
+                    var link = links[i];
                     if (link.Start > pos)
                         inlines.Add(new Run(message.Substring(pos, link.Start - pos)));
 
@@ -95,8 +96,7 @@ namespace SourceGit.Views
                 point = new Point(x, y);
 
                 var pos = TextLayout.HitTestPoint(point).TextPosition;
-
+                if (links.Intersect(pos, 1) is { } link)
-                if (links.Lookup(pos) is { } link)
                     SetHoveredIssueLink(link);
                 else
                     ClearHoveredIssueLink();

src/Views/CommitSubjectPresenter.cs

@@ -167,6 +167,9 @@ namespace SourceGit.Views
 
                     var start = match.Index;
                     var len = match.Length;
+                    if (_elements.Intersect(start, len) != null)
+                        continue;
+
                     _elements.Add(new Models.InlineElement(Models.InlineElementType.Code, start, len, string.Empty));
                 }
 
@@ -174,6 +177,7 @@ namespace SourceGit.Views
                 foreach (var rule in rules)
                     rule.Matches(_elements, subject);
 
+                _elements.Sort();
                 _needRebuildInlines = true;
                 InvalidateVisual();
             }
@@ -247,8 +251,9 @@ namespace SourceGit.Views
             var codeTypeface = new Typeface(codeFontFamily, FontStyle.Normal, FontWeight);
             var pos = 0;
             var x = 0.0;
-            foreach (var elem in _elements)
+            for (var i = 0; i < _elements.Count; i++)
             {
+                var elem = _elements[i];
                 if (elem.Start > pos)
                 {
                     var normal = new FormattedText(
@@ -350,7 +355,7 @@ namespace SourceGit.Views
             }
         }
 
-        private Models.InlineElementCollector _elements = [];
+        private Models.InlineElementCollector _elements = new();
         private List<Inline> _inlines = [];
         private Models.InlineElement _lastHover = null;
         private bool _needRebuildInlines = false;