From 84fb39f97a23eb15c42460d5df8d990674664804 Mon Sep 17 00:00:00 2001 From: leo Date: Sun, 8 Jun 2025 11:09:20 +0800 Subject: [PATCH] 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 ` - sort whold list after all elements have been added to avoid unnecessary memmove in `Insert` Signed-off-by: leo --- src/Models/Commit.cs | 8 +-- src/Models/InlineElement.cs | 13 ++-- src/Models/InlineElementCollector.cs | 97 +++++++--------------------- src/Models/IssueTrackerRule.cs | 2 + src/ViewModels/CommitDetail.cs | 5 ++ src/Views/CommitMessagePresenter.cs | 6 +- src/Views/CommitSubjectPresenter.cs | 9 ++- 7 files changed, 52 insertions(+), 88 deletions(-) diff --git a/src/Models/Commit.cs b/src/Models/Commit.cs index 6bf2655a..865b3ac1 100644 --- a/src/Models/Commit.cs +++ b/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 Parents { get; set; } = new List(); - public List Decorators { get; set; } = new List(); + public List Parents { get; set; } = new(); + public List 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(); } } diff --git a/src/Models/InlineElement.cs b/src/Models/InlineElement.cs index 53761403..ea7bcee8 100644 --- a/src/Models/InlineElement.cs +++ b/src/Models/InlineElement.cs @@ -2,8 +2,7 @@ { public enum InlineElementType { - None = 0, - Keyword, + Keyword = 0, Link, CommitSHA, Code, @@ -11,10 +10,10 @@ 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 @@ Link = link; } - public bool Intersect(int start, int length) + public bool IsIntersecting(int start, int length) { if (start == Start) return true; diff --git a/src/Models/InlineElementCollector.cs b/src/Models/InlineElementCollector.cs index b4b2e9e1..d81aaf8d 100644 --- a/src/Models/InlineElementCollector.cs +++ b/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 + public class InlineElementCollector { - private readonly List _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; - - 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 GetEnumerator() => _implementation.GetEnumerator(); - - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + private readonly List _implementation = []; } } diff --git a/src/Models/IssueTrackerRule.cs b/src/Models/IssueTrackerRule.cs index 30bce596..40c84b9e 100644 --- a/src/Models/IssueTrackerRule.cs +++ b/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++) diff --git a/src/ViewModels/CommitDetail.cs b/src/ViewModels/CommitDetail.cs index 8cc18152..8d2ade09 100644 --- a/src/ViewModels/CommitDetail.cs +++ b/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; } diff --git a/src/Views/CommitMessagePresenter.cs b/src/Views/CommitMessagePresenter.cs index 87433c45..d7c7ee3f 100644 --- a/src/Views/CommitMessagePresenter.cs +++ b/src/Views/CommitMessagePresenter.cs @@ -48,8 +48,9 @@ namespace SourceGit.Views var inlines = new List(); 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.Lookup(pos) is { } link) + if (links.Intersect(pos, 1) is { } link) SetHoveredIssueLink(link); else ClearHoveredIssueLink(); diff --git a/src/Views/CommitSubjectPresenter.cs b/src/Views/CommitSubjectPresenter.cs index b860ec1d..361c1184 100644 --- a/src/Views/CommitSubjectPresenter.cs +++ b/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 _inlines = []; private Models.InlineElement _lastHover = null; private bool _needRebuildInlines = false;