Lazily enumerate tree entry changes

Author: niik

LibGit2Sharp/TreeChanges.cs

@@ -17,32 +17,8 @@ namespace LibGit2Sharp
     [DebuggerDisplay("{DebuggerDisplay,nq}")]
     public class TreeChanges : IEnumerable<TreeEntryChanges>, IDiffResult
     {
-        private readonly List<TreeEntryChanges> changes = new List<TreeEntryChanges>();
-        private readonly List<TreeEntryChanges> added = new List<TreeEntryChanges>();
-        private readonly List<TreeEntryChanges> deleted = new List<TreeEntryChanges>();
-        private readonly List<TreeEntryChanges> modified = new List<TreeEntryChanges>();
-        private readonly List<TreeEntryChanges> typeChanged = new List<TreeEntryChanges>();
-        private readonly List<TreeEntryChanges> unmodified = new List<TreeEntryChanges>();
-        private readonly List<TreeEntryChanges> renamed = new List<TreeEntryChanges>();
-        private readonly List<TreeEntryChanges> copied = new List<TreeEntryChanges>();
-        private readonly List<TreeEntryChanges> conflicted = new List<TreeEntryChanges>();
-
-        private readonly IDictionary<ChangeKind, Action<TreeChanges, TreeEntryChanges>> fileDispatcher = Build();
-
-        private static IDictionary<ChangeKind, Action<TreeChanges, TreeEntryChanges>> Build()
-        {
-            return new Dictionary<ChangeKind, Action<TreeChanges, TreeEntryChanges>>
-            {
-                { ChangeKind.Modified,    (de, d) => de.modified.Add(d) },
-                { ChangeKind.Deleted,     (de, d) => de.deleted.Add(d) },
-                { ChangeKind.Added,       (de, d) => de.added.Add(d) },
-                { ChangeKind.TypeChanged, (de, d) => de.typeChanged.Add(d) },
-                { ChangeKind.Unmodified,  (de, d) => de.unmodified.Add(d) },
-                { ChangeKind.Renamed,     (de, d) => de.renamed.Add(d) },
-                { ChangeKind.Copied,      (de, d) => de.copied.Add(d) },
-                { ChangeKind.Conflicted,  (de, d) => de.conflicted.Add(d) },
-            };
-        }
+        private readonly DiffSafeHandle diff;
+        private readonly Lazy<int> count;
 
         /// <summary>
         /// Needed for mocking purposes.
@@ -52,22 +28,28 @@ protected TreeChanges()
 
         internal TreeChanges(DiffSafeHandle diff)
         {
-            using(diff)
-                Proxy.git_diff_foreach(diff, FileCallback, null, null);
+            this.diff = diff;
+            this.count = new Lazy<int>(() => Proxy.git_diff_num_deltas(diff));
         }
 
-        private int FileCallback(GitDiffDelta delta, float progress, IntPtr payload)
+        /// <summary>
+        /// Enumerates the diff and yields deltas with the specified change kind.
+        /// </summary>
+        /// <param name="changeKind">Change type to filter on.</param>
+        private IEnumerable<TreeEntryChanges> GetChangesOfKind(ChangeKind changeKind)
         {
-            AddFileChange(delta);
-            return 0;
+            for (int i = 0; i < count.Value; i++)
+            {
+                var delta = Proxy.git_diff_get_delta(diff, i);
+
+                if (TreeEntryChanges.GetStatusFromChangeKind(delta.Status) == changeKind)
+                    yield return new TreeEntryChanges(delta);
+            }
         }
 
-        private void AddFileChange(GitDiffDelta delta)
+        private static TreeEntryChanges ToTreeEntryChange(GitDiffDelta delta)
         {
-            var treeEntryChanges = new TreeEntryChanges(delta);
-
-            fileDispatcher[treeEntryChanges.Status](this, treeEntryChanges);
-            changes.Add(treeEntryChanges);
+            return new TreeEntryChanges(delta);
         }
 
         #region IEnumerable<TreeEntryChanges> Members
@@ -78,7 +60,10 @@ private void AddFileChange(GitDiffDelta delta)
         /// <returns>An <see cref="IEnumerator{T}"/> object that can be used to iterate through the collection.</returns>
         public virtual IEnumerator<TreeEntryChanges> GetEnumerator()
         {
-            return changes.GetEnumerator();
+            for (int i = 0; i < count.Value; i++)
+            {
+                yield return new TreeEntryChanges(Proxy.git_diff_get_delta(diff, i));
+            }
         }
 
         /// <summary>
@@ -97,63 +82,63 @@ IEnumerator IEnumerable.GetEnumerator()
         /// </summary>
         public virtual IEnumerable<TreeEntryChanges> Added
         {
-            get { return added; }
+            get { return GetChangesOfKind(ChangeKind.Added); }
         }
 
         /// <summary>
         /// List of <see cref="TreeEntryChanges"/> that have been deleted.
         /// </summary>
         public virtual IEnumerable<TreeEntryChanges> Deleted
         {
-            get { return deleted; }
+            get { return GetChangesOfKind(ChangeKind.Deleted); }
         }
 
         /// <summary>
         /// List of <see cref="TreeEntryChanges"/> that have been modified.
         /// </summary>
         public virtual IEnumerable<TreeEntryChanges> Modified
         {
-            get { return modified; }
+            get { return GetChangesOfKind(ChangeKind.Modified); }
         }
 
         /// <summary>
         /// List of <see cref="TreeEntryChanges"/> which type have been changed.
         /// </summary>
         public virtual IEnumerable<TreeEntryChanges> TypeChanged
         {
-            get { return typeChanged; }
+            get { return GetChangesOfKind(ChangeKind.TypeChanged); }
         }
 
         /// <summary>
         /// List of <see cref="TreeEntryChanges"/> which have been renamed
         /// </summary>
         public virtual IEnumerable<TreeEntryChanges> Renamed
         {
-            get { return renamed; }
+            get { return GetChangesOfKind(ChangeKind.Renamed); }
         }
 
         /// <summary>
         /// List of <see cref="TreeEntryChanges"/> which have been copied
         /// </summary>
         public virtual IEnumerable<TreeEntryChanges> Copied
         {
-            get { return copied; }
+            get { return GetChangesOfKind(ChangeKind.Copied); }
         }
 
         /// <summary>
         /// List of <see cref="TreeEntryChanges"/> which are unmodified
         /// </summary>
         public virtual IEnumerable<TreeEntryChanges> Unmodified
         {
-            get { return unmodified; }
+            get { return GetChangesOfKind(ChangeKind.Unmodified); }
         }
 
         /// <summary>
         /// List of <see cref="TreeEntryChanges"/> which are conflicted
         /// </summary>
         public virtual IEnumerable<TreeEntryChanges> Conflicted
         {
-            get { return conflicted; }
+            get { return GetChangesOfKind(ChangeKind.Conflicted); }
         }
 
         private string DebuggerDisplay
@@ -179,8 +164,7 @@ public void Dispose()
 
         protected virtual void Dispose(bool disposing)
         {
-            // This doesn't do anything yet because it loads everything
-            // eagerly and disposes of the diff handle in the constructor.
+            diff.SafeDispose();
         }
     }
 }

LibGit2Sharp/TreeEntryChanges.cs

@@ -28,9 +28,23 @@ internal TreeEntryChanges(GitDiffDelta delta)
             Exists = (delta.NewFile.Flags & GitDiffFlags.GIT_DIFF_FLAG_EXISTS) != 0;
             OldExists = (delta.OldFile.Flags & GitDiffFlags.GIT_DIFF_FLAG_EXISTS) != 0;
 
-            Status = (delta.Status == ChangeKind.Untracked || delta.Status == ChangeKind.Ignored)
-                ? ChangeKind.Added
-                : delta.Status;
+            Status = GetStatusFromChangeKind(delta.Status);
+        }
+
+        // This treatment of change kind was apparently introduced in order to be able
+        // to compare a tree against the index, see commit fdc972b. It's extracted
+        // here so that TreeEntry can use the same rules without having to instantiate
+        // a TreeEntryChanges object.
+        internal static ChangeKind GetStatusFromChangeKind(ChangeKind changeKind)
+        {
+            switch (changeKind)
+            {
+                case ChangeKind.Untracked:
+                case ChangeKind.Ignored:
+                    return ChangeKind.Added;
+                default:
+                    return changeKind;
+            }
         }
 
         /// <summary>