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 <[email protected]>

Author: love-linger

src/Models/Commit.cs

@@ -33,8 +33,8 @@ public static double OpacityForNotMerged
         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<Decorator> Decorators { get; set; } = new List<Decorator>();
+        public List<string> Parents { get; set; } = new();
+        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 @@ public static double OpacityForNotMerged
         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 @@ public void ParseDecorators(string data)
     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,19 +2,18 @@
 {
     public enum InlineElementType
     {
-        None = 0,
-        Keyword,
+        Keyword = 0,
         Link,
         CommitSHA,
         Code,
     }
 
     public class InlineElement
     {
-        public InlineElementType Type { get; set; } = InlineElementType.None;
-        public int Start { get; set; } = 0;
-        public int Length { get; set; } = 0;
-        public string Link { get; set; } = "";
+        public InlineElementType Type { get; }
+        public int Start { get; }
+        public int Length { get; }
+        public string Link { get; }
 
         public InlineElement(InlineElementType type, int start, int length, string link)
         {
@@ -24,7 +23,7 @@ public InlineElement(InlineElementType type, int start, int length, string link)
             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 void Clear()
-        {
-            _implementation.Clear();
-
-            AssertInvariant();
-        }
-
         public int Count => _implementation.Count;
+        public InlineElement this[int index] => _implementation[index];
 
-        public void Add(InlineElement element)
+        public InlineElement Intersect(int start, int length)
         {
-
-            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++)
+            foreach (var elem in _implementation)
             {
-                var prev = _implementation[index - 1];
-                var curr = _implementation[index];
-
-                Debug.Assert(prev.Start + prev.Length <= curr.Start);
+                if (elem.IsIntersecting(start, length))
+                    return elem;
             }
+
+            return null;
         }
 
-        public InlineElement Lookup(int position)
+        public void Add(InlineElement element)
         {
-            var index = FindIndex(position);
-            return IsIntersection(index, position, 1)
-                ? _implementation[index]
-                : null;
+            _implementation.Add(element);
         }
 
-        private int FindIndex(int start)
+        public void Sort()
         {
-            var index = 0;
-            while (index < _implementation.Count && _implementation[index].Start <= start)
-                index++;
-
-            return index;
+            _implementation.Sort((l, r) => l.Start.CompareTo(r.Start));
         }
 
-        private bool IsIntersection(int index, int start, int length)
+        public void Clear()
         {
-            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;
+            _implementation.Clear();
         }
 
-        public IEnumerator<InlineElement> GetEnumerator() => _implementation.GetEnumerator();
-
-        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+        private readonly List<InlineElement> _implementation = [];
     }
 }

src/Models/IssueTrackerRule.cs

@@ -59,6 +59,8 @@ public void Matches(InlineElementCollector outs, string message)
 
                 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 @@ private Models.InlineElementCollector ParseInlinesInMessage(string message)
 
                 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,13 +655,16 @@ private Models.InlineElementCollector ParseInlinesInMessage(string message)
 
                 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();
                 if (isCommitSHA)
                     inlines.Add(new Models.InlineElement(Models.InlineElementType.CommitSHA, start, len, sha));
             }
 
+            inlines.Sort();
             return inlines;
         }
 

src/Views/CommitMessagePresenter.cs

@@ -48,8 +48,9 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs chang
 
                 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 @@ protected override void OnPointerMoved(PointerEventArgs e)
                 point = new Point(x, y);
 
                 var pos = TextLayout.HitTestPoint(point).TextPosition;
-
-                if (links.Lookup(pos) is { } link)
+                if (links.Intersect(pos, 1) is { } link)
                     SetHoveredIssueLink(link);
                 else
                     ClearHoveredIssueLink();

src/Views/CommitSubjectPresenter.cs

@@ -167,13 +167,17 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs chang
 
                     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));
                 }
 
                 var rules = IssueTrackerRules ?? [];
                 foreach (var rule in rules)
                     rule.Matches(_elements, subject);
 
+                _elements.Sort();
                 _needRebuildInlines = true;
                 InvalidateVisual();
             }
@@ -247,8 +251,9 @@ private void GenerateFormattedTextElements()
             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 @@ public Inline(double x, FormattedText text, Models.InlineElement elem)
             }
         }
 
-        private Models.InlineElementCollector _elements = [];
+        private Models.InlineElementCollector _elements = new();
         private List<Inline> _inlines = [];
         private Models.InlineElement _lastHover = null;
         private bool _needRebuildInlines = false;