Merge branch 'fixes/changes-view-refactoring' into fixes/changes-view-performance

StanleyGoldman · StanleyGoldman · commit b26560bffc4e · 2017-07-13T12:27:33.000-04:00
# Conflicts:
#	src/GitHub.Api/UI/TreeBuilder.cs
diff --git a/README.md b/README.md
@@ -86,7 +86,7 @@ The extension log file can be found at `~/.local/share/GitHubUnityDebug/github-u
 
 ##### Windows
 
-The extension log file can be found at `%LOCALAPPDATA%\GitHubUnityDebug\github-unity.log`
+The extension log file can be found at `%LOCALAPPDATA%\GitHubUnity\github-unity.log`
 
 ## Building and Contributing
 
diff --git a/src/GitHub.Api/UI/FileTreeNode.cs b/src/GitHub.Api/UI/FileTreeNode.cs
@@ -1,4 +1,3 @@
-using System;
 using System.Collections.Generic;
 using System.Linq;
 
@@ -13,7 +12,6 @@ enum CommitState
 
     class FileTreeNode
     {
-        private readonly Action<FileTreeNode> stateChangeCallback;
         private List<FileTreeNode> children;
         private string path;
         private CommitState state;
@@ -24,15 +22,13 @@ class FileTreeNode
         public string RepositoryPath;
         public GitCommitTarget Target { get; set; }
 
-        public FileTreeNode(Action<FileTreeNode> stateChangeCallback)
+        public FileTreeNode()
         {
-            this.stateChangeCallback = stateChangeCallback;
             children = new List<FileTreeNode>();
         }
 
-        public FileTreeNode(string path, Action<FileTreeNode> stateChangeCallback)
+        public FileTreeNode(string path)
         {
-            this.stateChangeCallback = stateChangeCallback;
             this.path = path ?? "";
             Label = this.path;
             children = new List<FileTreeNode>();
@@ -106,7 +102,6 @@ public CommitState State
                 }
 
                 state = value;
-                stateChangeCallback?.Invoke(this);
 
                 if (children == null)
                 {
diff --git a/src/GitHub.Api/UI/TreeBuilder.cs b/src/GitHub.Api/UI/TreeBuilder.cs
@@ -6,7 +6,7 @@ namespace GitHub.Unity
 {
     static class TreeBuilder
     {
-        internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, HashSet<string> foldedTreeSet, Action<FileTreeNode> stateChangeCallback)
+        internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, HashSet<string> foldedTreeSet)
         {
             if (String.IsNullOrEmpty(node.Label))
             {
@@ -33,7 +33,7 @@ internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, Hash
                     if (child.Label.Equals(root))
                     {
                         found = true;
-                        BuildChildNode(child, node, foldedTreeSet, stateChangeCallback);
+                        BuildChildNode(child, node, foldedTreeSet);
                         break;
                     }
                 }
@@ -42,7 +42,7 @@ internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, Hash
                 if (!found)
                 {
                     var p = parent.RepositoryPath.ToNPath().Combine(root);
-                    BuildChildNode(parent.Add(new FileTreeNode(root, stateChangeCallback) { RepositoryPath = p }), node, foldedTreeSet, stateChangeCallback);
+                    BuildChildNode(parent.Add(new FileTreeNode(root) { RepositoryPath = p }), node, foldedTreeSet);
                 }
             }
             else if (nodePath.ExtensionWithDot == ".meta")
@@ -56,7 +56,7 @@ internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, Hash
                     if (child.Label.Equals(searchLabel))
                     {
                         found = true;
-                        BuildChildNode(child, node, foldedTreeSet, stateChangeCallback);
+                        BuildChildNode(child, node, foldedTreeSet);
                         break;
                     }
                 }
@@ -72,7 +72,7 @@ internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, Hash
             }
         }
 
-        internal static FileTreeNode BuildTreeRoot(IList<GitStatusEntry> newEntries, List<GitStatusEntry> gitStatusEntries, List<GitCommitTarget> gitCommitTargets, List<string> foldedTreeEntries, Action<FileTreeNode> stateChangeCallback, Func<string, object> iconLoaderFunc = null)
+        internal static FileTreeNode BuildTreeRoot(IList<GitStatusEntry> newEntries, List<GitStatusEntry> gitStatusEntries, List<GitCommitTarget> gitCommitTargets, List<string> foldedTreeEntries, Func<string, object> iconLoaderFunc = null)
         {
             Guard.ArgumentNotNullOrEmpty(newEntries, "newEntries");
 
@@ -123,7 +123,7 @@ internal static FileTreeNode BuildTreeRoot(IList<GitStatusEntry> newEntries, Lis
             // TODO: In stead of completely rebuilding the tree structure, figure out a way to migrate open/closed states from the old tree to the new
             // Build tree structure
 
-            var tree = new FileTreeNode(FileSystemHelpers.FindCommonPath(gitStatusEntries.Select(e => e.Path)), stateChangeCallback);
+            var tree = new FileTreeNode(FileSystemHelpers.FindCommonPath(gitStatusEntries.Select(e => e.Path)));
             tree.RepositoryPath = tree.Path;
 
             for (var index1 = 0; index1 < gitStatusEntries.Count; index1++)
@@ -132,13 +132,13 @@ internal static FileTreeNode BuildTreeRoot(IList<GitStatusEntry> newEntries, Lis
                 var entryPath = gitStatusEntry.Path.ToNPath();
                 if (entryPath.IsChildOf(tree.Path)) entryPath = entryPath.RelativeTo(tree.Path.ToNPath());
 
-                var node = new FileTreeNode(entryPath, stateChangeCallback) { Target = gitCommitTargets[index1] };
+                var node = new FileTreeNode(entryPath) { Target = gitCommitTargets[index1] };
                 if (!String.IsNullOrEmpty(gitStatusEntry.ProjectPath))
                 {
                     node.Icon = iconLoaderFunc?.Invoke(gitStatusEntry.ProjectPath);
                 }
 
-                BuildChildNode(tree, node, foldedTreeSet, stateChangeCallback);
+                BuildChildNode(tree, node, foldedTreeSet);
             }
 
             return tree;
diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/ChangesetTreeView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/ChangesetTreeView.cs
@@ -19,7 +19,6 @@ class ChangesetTreeView : Subview
         [SerializeField] private List<GitCommitTarget> entryCommitTargets = new List<GitCommitTarget>();
         [SerializeField] private List<string> foldedTreeEntries = new List<string>();
         [NonSerialized] private FileTreeNode tree;
-        [NonSerialized] private Action<FileTreeNode> stateChangeCallback;
 
         public override void OnGUI()
         {
@@ -109,7 +108,7 @@ public void UpdateEntries(IList<GitStatusEntry> newEntries)
                 return;
             }
 
-            tree = TreeBuilder.BuildTreeRoot(newEntries, entries, entryCommitTargets, foldedTreeEntries, stateChangeCallback, AssetDatabase.GetCachedIcon);
+            tree = TreeBuilder.BuildTreeRoot(newEntries, entries, entryCommitTargets, foldedTreeEntries, AssetDatabase.GetCachedIcon);
 
             OnCommitTreeChange();
         }
@@ -135,25 +134,7 @@ private void TreeNode(FileTreeNode node)
                     }
                     if (EditorGUI.EndChangeCheck())
                     {
-                        var filesAdded = new List<string>();
-                        var filesRemoved = new List<string>();
-                        stateChangeCallback = new Action<FileTreeNode>(s =>
-                        {
-                            if (s.State == CommitState.None)
-                                filesRemoved.Add(s.Path);
-                            else
-                                filesAdded.Add(s.Path);
-                        });
                         node.State = toggled ? CommitState.All : CommitState.None;
-                        if (filesAdded.Count > 0)
-                            GitClient.Add(filesAdded);
-                        if (filesRemoved.Count > 0)
-                            GitClient.Remove(filesAdded);
-                        if (filesAdded.Count > 0|| filesRemoved.Count > 0)
-                        {
-                            GitClient.Status();
-                        }
-                        // we might need to run git status after these calls
                     }
                 }
 
diff --git a/src/tests/UnitTests/UI/TreeBuilderTests.cs b/src/tests/UnitTests/UI/TreeBuilderTests.cs
@@ -33,7 +33,7 @@ public void CanBuildTreeForSingleItem()
             };
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(1);
             gitCommitTargets.Count.Should().Be(1);
@@ -71,7 +71,7 @@ public void CanBuildTreeForSingleItemWhenProjectNestedInRepo()
             };
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(1);
             gitCommitTargets.Count.Should().Be(1);
@@ -107,7 +107,7 @@ public void CanBuildTreeForSingleItemInFolder()
             };
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(1);
             gitCommitTargets.Count.Should().Be(1);
@@ -143,7 +143,7 @@ public void CanBuildTreeForSingleItemInFolderWhenProjectNestedInRepo()
             };
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(1);
             gitCommitTargets.Count.Should().Be(1);
@@ -180,7 +180,7 @@ public void CanBuildTreeForMultipleSiblings()
             };
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(2);
             gitCommitTargets.Count.Should().Be(2);
@@ -228,7 +228,7 @@ public void CanBuildTreeForMultipleSiblingsWhenProjectNestedInRepo()
             };
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(2);
             gitCommitTargets.Count.Should().Be(2);
@@ -279,7 +279,7 @@ public void CanBuildTreeForHierarchy()
             };
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(5);
             gitCommitTargets.Count.Should().Be(5);
@@ -380,7 +380,7 @@ public void CanBuildTreeForHierarchyWhenProjectNestedInRepo()
             };
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(2);
             gitCommitTargets.Count.Should().Be(2);
@@ -439,7 +439,7 @@ public void CanBuildTreeForItemAndMetafile()
             };
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(2);
             gitCommitTargets.Count.Should().Be(2);
@@ -493,7 +493,7 @@ public void CanUpdateTreeAndMaintainSelectedState()
             var stateChangeCallbackListener = Substitute.For<IStateChangeCallbackListener>();
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries1, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(2);
             gitCommitTargets.Count.Should().Be(2);
@@ -533,7 +533,7 @@ public void CanUpdateTreeAndMaintainSelectedState()
             };
 
             treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries2, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             treeRootChidren = treeRoot.Children.ToArray();
             treeRootChidren.Length.Should().Be(2);
@@ -584,7 +584,7 @@ public void CanUpdateTreeAndMaintainFoldedState()
             var stateChangeCallbackListener = Substitute.For<IStateChangeCallbackListener>();
 
             var treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(5);
             gitCommitTargets.Count.Should().Be(5);
@@ -681,7 +681,7 @@ public void CanUpdateTreeAndMaintainFoldedState()
             foldedTreeEntries.Count.Should().Be(2);
 
             treeRoot = TreeBuilder.BuildTreeRoot(newGitStatusEntries, gitStatusEntries, gitCommitTargets,
-                foldedTreeEntries, stateChangeCallbackListener.StateChangeCallback);
+                foldedTreeEntries);
 
             gitStatusEntries.Count.Should().Be(4);
             gitCommitTargets.Count.Should().Be(4);

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-using System;`
`2`	`1`	`using System.Collections.Generic;`
`3`	`2`	`using System.Linq;`
`4`	`3`
`@@ -13,7 +12,6 @@ enum CommitState`
`13`	`12`
`14`	`13`	`class FileTreeNode`
`15`	`14`	`{`
`16`		`- private readonly Action<FileTreeNode> stateChangeCallback;`
`17`	`15`	`private List<FileTreeNode> children;`
`18`	`16`	`private string path;`
`19`	`17`	`private CommitState state;`
`@@ -24,15 +22,13 @@ class FileTreeNode`
`24`	`22`	`public string RepositoryPath;`
`25`	`23`	`public GitCommitTarget Target { get; set; }`
`26`	`24`
`27`		`- public FileTreeNode(Action<FileTreeNode> stateChangeCallback)`
	`25`	`+ public FileTreeNode()`
`28`	`26`	`{`
`29`		`- this.stateChangeCallback = stateChangeCallback;`
`30`	`27`	`children = new List<FileTreeNode>();`
`31`	`28`	`}`
`32`	`29`
`33`		`- public FileTreeNode(string path, Action<FileTreeNode> stateChangeCallback)`
	`30`	`+ public FileTreeNode(string path)`
`34`	`31`	`{`
`35`		`- this.stateChangeCallback = stateChangeCallback;`
`36`	`32`	`this.path = path ?? "";`
`37`	`33`	`Label = this.path;`
`38`	`34`	`children = new List<FileTreeNode>();`
`@@ -106,7 +102,6 @@ public CommitState State`
`106`	`102`	`}`
`107`	`103`
`108`	`104`	`state = value;`
`109`		`- stateChangeCallback?.Invoke(this);`
`110`	`105`
`111`	`106`	`if (children == null)`
`112`	`107`	`{`
Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@ namespace GitHub.Unity`
`6`	`6`	`{`
`7`	`7`	`static class TreeBuilder`
`8`	`8`	`{`
`9`		`- internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, HashSet<string> foldedTreeSet, Action<FileTreeNode> stateChangeCallback)`
	`9`	`+ internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, HashSet<string> foldedTreeSet)`
`10`	`10`	`{`
`11`	`11`	`if (String.IsNullOrEmpty(node.Label))`
`12`	`12`	`{`
`@@ -33,7 +33,7 @@ internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, Hash`
`33`	`33`	`if (child.Label.Equals(root))`
`34`	`34`	`{`
`35`	`35`	`found = true;`
`36`		`- BuildChildNode(child, node, foldedTreeSet, stateChangeCallback);`
	`36`	`+ BuildChildNode(child, node, foldedTreeSet);`
`37`	`37`	`break;`
`38`	`38`	`}`
`39`	`39`	`}`
`@@ -42,7 +42,7 @@ internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, Hash`
`42`	`42`	`if (!found)`
`43`	`43`	`{`
`44`	`44`	`var p = parent.RepositoryPath.ToNPath().Combine(root);`
`45`		`- BuildChildNode(parent.Add(new FileTreeNode(root, stateChangeCallback) { RepositoryPath = p }), node, foldedTreeSet, stateChangeCallback);`
	`45`	`+ BuildChildNode(parent.Add(new FileTreeNode(root) { RepositoryPath = p }), node, foldedTreeSet);`
`46`	`46`	`}`
`47`	`47`	`}`
`48`	`48`	`else if (nodePath.ExtensionWithDot == ".meta")`
`@@ -56,7 +56,7 @@ internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, Hash`
`56`	`56`	`if (child.Label.Equals(searchLabel))`
`57`	`57`	`{`
`58`	`58`	`found = true;`
`59`		`- BuildChildNode(child, node, foldedTreeSet, stateChangeCallback);`
	`59`	`+ BuildChildNode(child, node, foldedTreeSet);`
`60`	`60`	`break;`
`61`	`61`	`}`
`62`	`62`	`}`
`@@ -72,7 +72,7 @@ internal static void BuildChildNode(FileTreeNode parent, FileTreeNode node, Hash`
`72`	`72`	`}`
`73`	`73`	`}`
`74`	`74`
`75`		`- internal static FileTreeNode BuildTreeRoot(IList<GitStatusEntry> newEntries, List<GitStatusEntry> gitStatusEntries, List<GitCommitTarget> gitCommitTargets, List<string> foldedTreeEntries, Action<FileTreeNode> stateChangeCallback, Func<string, object> iconLoaderFunc = null)`
	`75`	`+ internal static FileTreeNode BuildTreeRoot(IList<GitStatusEntry> newEntries, List<GitStatusEntry> gitStatusEntries, List<GitCommitTarget> gitCommitTargets, List<string> foldedTreeEntries, Func<string, object> iconLoaderFunc = null)`
`76`	`76`	`{`
`77`	`77`	`Guard.ArgumentNotNullOrEmpty(newEntries, "newEntries");`
`78`	`78`
`@@ -123,7 +123,7 @@ internal static FileTreeNode BuildTreeRoot(IList<GitStatusEntry> newEntries, Lis`
`123`	`123`	`// TODO: In stead of completely rebuilding the tree structure, figure out a way to migrate open/closed states from the old tree to the new`
`124`	`124`	`// Build tree structure`
`125`	`125`
`126`		`- var tree = new FileTreeNode(FileSystemHelpers.FindCommonPath(gitStatusEntries.Select(e => e.Path)), stateChangeCallback);`
	`126`	`+ var tree = new FileTreeNode(FileSystemHelpers.FindCommonPath(gitStatusEntries.Select(e => e.Path)));`
`127`	`127`	`tree.RepositoryPath = tree.Path;`
`128`	`128`
`129`	`129`	`for (var index1 = 0; index1 < gitStatusEntries.Count; index1++)`
`@@ -132,13 +132,13 @@ internal static FileTreeNode BuildTreeRoot(IList<GitStatusEntry> newEntries, Lis`
`132`	`132`	`var entryPath = gitStatusEntry.Path.ToNPath();`
`133`	`133`	`if (entryPath.IsChildOf(tree.Path)) entryPath = entryPath.RelativeTo(tree.Path.ToNPath());`
`134`	`134`
`135`		`- var node = new FileTreeNode(entryPath, stateChangeCallback) { Target = gitCommitTargets[index1] };`
	`135`	`+ var node = new FileTreeNode(entryPath) { Target = gitCommitTargets[index1] };`
`136`	`136`	`if (!String.IsNullOrEmpty(gitStatusEntry.ProjectPath))`
`137`	`137`	`{`
`138`	`138`	`node.Icon = iconLoaderFunc?.Invoke(gitStatusEntry.ProjectPath);`
`139`	`139`	`}`
`140`	`140`
`141`		`- BuildChildNode(tree, node, foldedTreeSet, stateChangeCallback);`
	`141`	`+ BuildChildNode(tree, node, foldedTreeSet);`
`142`	`142`	`}`
`143`	`143`
`144`	`144`	`return tree;`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,6 @@ class ChangesetTreeView : Subview`
`19`	`19`	`[SerializeField] private List<GitCommitTarget> entryCommitTargets = new List<GitCommitTarget>();`
`20`	`20`	`[SerializeField] private List<string> foldedTreeEntries = new List<string>();`
`21`	`21`	`[NonSerialized] private FileTreeNode tree;`
`22`		`- [NonSerialized] private Action<FileTreeNode> stateChangeCallback;`
`23`	`22`
`24`	`23`	`public override void OnGUI()`
`25`	`24`	`{`
`@@ -109,7 +108,7 @@ public void UpdateEntries(IList<GitStatusEntry> newEntries)`
`109`	`108`	`return;`
`110`	`109`	`}`
`111`	`110`
`112`		`- tree = TreeBuilder.BuildTreeRoot(newEntries, entries, entryCommitTargets, foldedTreeEntries, stateChangeCallback, AssetDatabase.GetCachedIcon);`
	`111`	`+ tree = TreeBuilder.BuildTreeRoot(newEntries, entries, entryCommitTargets, foldedTreeEntries, AssetDatabase.GetCachedIcon);`
`113`	`112`
`114`	`113`	`OnCommitTreeChange();`
`115`	`114`	`}`
`@@ -135,25 +134,7 @@ private void TreeNode(FileTreeNode node)`
`135`	`134`	`}`
`136`	`135`	`if (EditorGUI.EndChangeCheck())`
`137`	`136`	`{`
`138`		`- var filesAdded = new List<string>();`
`139`		`- var filesRemoved = new List<string>();`
`140`		`- stateChangeCallback = new Action<FileTreeNode>(s =>`
`141`		`- {`
`142`		`- if (s.State == CommitState.None)`
`143`		`- filesRemoved.Add(s.Path);`
`144`		`- else`
`145`		`- filesAdded.Add(s.Path);`
`146`		`- });`
`147`	`137`	`node.State = toggled ? CommitState.All : CommitState.None;`
`148`		`- if (filesAdded.Count > 0)`
`149`		`- GitClient.Add(filesAdded);`
`150`		`- if (filesRemoved.Count > 0)`
`151`		`- GitClient.Remove(filesAdded);`
`152`		`- if (filesAdded.Count > 0\|\| filesRemoved.Count > 0)`
`153`		`- {`
`154`		`- GitClient.Status();`
`155`		`- }`
`156`		`- // we might need to run git status after these calls`
`157`	`138`	`}`
`158`	`139`	`}`
`159`	`140`