Skip to content

Initial change to type-safe Diff.Compare #1180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 24, 2015
Merged

Initial change to type-safe Diff.Compare #1180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions LibGit2Sharp.Tests/DiffTreeToTreeFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1123,17 +1123,6 @@ public void RetrievingDiffChangesMustAlwaysBeCaseSensitive()
}
}

[Fact]
public void CallingCompareWithAnUnsupportedGenericParamThrows()
{
var path = SandboxStandardTestRepoGitDir();
using (var repo = new Repository(path))
{
Assert.Throws<LibGit2SharpException>(() => repo.Diff.Compare<string>(default(Tree), default(Tree)));
Assert.Throws<LibGit2SharpException>(() => repo.Diff.Compare<string>());
}
}

[Fact]
public void UsingPatienceAlgorithmCompareOptionProducesPatienceDiff()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails to compile, to removed. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. That's a win! 😉

{
Expand Down
16 changes: 15 additions & 1 deletion LibGit2Sharp.Tests/MetaFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class MetaFixture
{
private static readonly HashSet<Type> explicitOnlyInterfaces = new HashSet<Type>
{
typeof(IBelongToARepository),
typeof(IBelongToARepository), typeof(IDiffResult),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this for an empty interface?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Otherwise the test will check whether all the public api of the class is the public API contract defined by the interface. Which is empty. So every method will end up being 'extra' and the test will fail.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

};

[Fact]
Expand Down Expand Up @@ -401,6 +401,20 @@ where method.IsDefined(typeof(ExtensionAttribute), false)
select method;
return query;
}

[Fact]
public void AllIDiffResultsAreInChangesBuilder()
{
var diff = typeof(Diff).GetField("ChangesBuilders", BindingFlags.NonPublic | BindingFlags.Static);
var changesBuilders = (System.Collections.IDictionary)diff.GetValue(null);

IEnumerable<Type> diffResults = typeof(Diff).Assembly.GetExportedTypes()
.Where(type => type.GetInterface("IDiffResult") != null);

var nonBuilderTypes = diffResults.Where(diffResult => !changesBuilders.Contains(diffResult));
Assert.False(nonBuilderTypes.Any(), "Classes which implement IDiffResult but are not registered under ChangesBuilders in Diff:" + Environment.NewLine +
string.Join(Environment.NewLine, nonBuilderTypes.Select(type => type.FullName)));
}
}

internal static class TypeExtensions
Expand Down
86 changes: 34 additions & 52 deletions LibGit2Sharp/Diff.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,21 @@ private static IDictionary<DiffTargets, Func<Repository, TreeComparisonHandleRet
{ typeof(PatchStats), diff => new PatchStats(diff) },
};


private static T BuildDiffResult<T>(DiffSafeHandle diff) where T : class, IDiffResult
{
Func<DiffSafeHandle, object> builder;

if (!ChangesBuilders.TryGetValue(typeof(T), out builder))
{
throw new LibGit2SharpException(CultureInfo.InvariantCulture,
"User-defined types passed to Compare are not supported. Supported values are: {0}",
string.Join(", ", ChangesBuilders.Keys.Select(x => x.Name)));
}

return (T)builder(diff);
}

/// <summary>
/// Show changes between two <see cref="Blob"/>s.
/// </summary>
Expand Down Expand Up @@ -134,7 +149,7 @@ public virtual ContentChanges Compare(Blob oldBlob, Blob newBlob, CompareOptions
/// <param name="oldTree">The <see cref="Tree"/> you want to compare from.</param>
/// <param name="newTree">The <see cref="Tree"/> you want to compare to.</param>
/// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
public virtual T Compare<T>(Tree oldTree, Tree newTree) where T : class
public virtual T Compare<T>(Tree oldTree, Tree newTree) where T : class, IDiffResult
{
return Compare<T>(oldTree, newTree, null, null, null);
}
Expand All @@ -146,7 +161,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree) where T : class
/// <param name="newTree">The <see cref="Tree"/> you want to compare to.</param>
/// <param name="paths">The list of paths (either files or directories) that should be compared.</param>
/// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths) where T : class
public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths) where T : class, IDiffResult
{
return Compare<T>(oldTree, newTree, paths, null, null);
}
Expand All @@ -163,7 +178,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> path
/// </param>
/// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths,
ExplicitPathsOptions explicitPathsOptions) where T : class
ExplicitPathsOptions explicitPathsOptions) where T : class, IDiffResult
{
return Compare<T>(oldTree, newTree, paths, explicitPathsOptions, null);
}
Expand All @@ -176,7 +191,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> path
/// <param name="paths">The list of paths (either files or directories) that should be compared.</param>
/// <param name="compareOptions">Additional options to define patch generation behavior.</param>
/// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths, CompareOptions compareOptions) where T : class
public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths, CompareOptions compareOptions) where T : class, IDiffResult
{
return Compare<T>(oldTree, newTree, paths, null, compareOptions);
}
Expand All @@ -188,7 +203,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> path
/// <param name="newTree">The <see cref="Tree"/> you want to compare to.</param>
/// <param name="compareOptions">Additional options to define patch generation behavior.</param>
/// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
public virtual T Compare<T>(Tree oldTree, Tree newTree, CompareOptions compareOptions) where T : class
public virtual T Compare<T>(Tree oldTree, Tree newTree, CompareOptions compareOptions) where T : class, IDiffResult
{
return Compare<T>(oldTree, newTree, null, null, compareOptions);
}
Expand All @@ -206,19 +221,8 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, CompareOptions compareOp
/// <param name="compareOptions">Additional options to define patch generation behavior.</param>
/// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths, ExplicitPathsOptions explicitPathsOptions,
CompareOptions compareOptions) where T : class
CompareOptions compareOptions) where T : class, IDiffResult
{
Func<DiffSafeHandle, object> builder;

if (!ChangesBuilders.TryGetValue(typeof(T), out builder))
{
throw new LibGit2SharpException(CultureInfo.InvariantCulture,
"Unexpected type '{0}' passed to Compare. Supported values are either '{1}' or '{2}'.",
typeof(T),
typeof(TreeChanges),
typeof(Patch));
}

var comparer = TreeToTree(repo);
ObjectId oldTreeId = oldTree != null ? oldTree.Id : null;
ObjectId newTreeId = newTree != null ? newTree.Id : null;
Expand All @@ -236,7 +240,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> path

using (DiffSafeHandle diff = BuildDiffList(oldTreeId, newTreeId, comparer, diffOptions, paths, explicitPathsOptions, compareOptions))
{
return (T)builder(diff);
return BuildDiffResult<T>(diff);
}
}

Expand All @@ -252,7 +256,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> path
/// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
/// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
/// <returns>A <typeparamref name="T"/> containing the changes between the <see cref="Tree"/> and the selected target.</returns>
public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets) where T : class
public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets) where T : class, IDiffResult
{
return Compare<T>(oldTree, diffTargets, null, null, null);
}
Expand All @@ -270,7 +274,7 @@ public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets) where T : cla
/// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
/// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
/// <returns>A <typeparamref name="T"/> containing the changes between the <see cref="Tree"/> and the selected target.</returns>
public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<string> paths) where T : class
public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<string> paths) where T : class, IDiffResult
{
return Compare<T>(oldTree, diffTargets, paths, null, null);
}
Expand All @@ -293,7 +297,7 @@ public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<s
/// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
/// <returns>A <typeparamref name="T"/> containing the changes between the <see cref="Tree"/> and the selected target.</returns>
public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<string> paths,
ExplicitPathsOptions explicitPathsOptions) where T : class
ExplicitPathsOptions explicitPathsOptions) where T : class, IDiffResult
{
return Compare<T>(oldTree, diffTargets, paths, explicitPathsOptions, null);
}
Expand All @@ -317,19 +321,8 @@ public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<s
/// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
/// <returns>A <typeparamref name="T"/> containing the changes between the <see cref="Tree"/> and the selected target.</returns>
public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<string> paths,
ExplicitPathsOptions explicitPathsOptions, CompareOptions compareOptions) where T : class
ExplicitPathsOptions explicitPathsOptions, CompareOptions compareOptions) where T : class, IDiffResult
{
Func<DiffSafeHandle, object> builder;

if (!ChangesBuilders.TryGetValue(typeof(T), out builder))
{
throw new LibGit2SharpException(CultureInfo.InvariantCulture,
"Unexpected type '{0}' passed to Compare. Supported values are either '{1}' or '{2}'.",
typeof(T),
typeof(TreeChanges),
typeof(Patch));
}

var comparer = HandleRetrieverDispatcher[diffTargets](repo);
ObjectId oldTreeId = oldTree != null ? oldTree.Id : null;

Expand All @@ -349,7 +342,7 @@ public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<s

using (DiffSafeHandle diff = BuildDiffList(oldTreeId, null, comparer, diffOptions, paths, explicitPathsOptions, compareOptions))
{
return (T)builder(diff);
return BuildDiffResult<T>(diff);
}
}

Expand All @@ -363,7 +356,7 @@ public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<s
/// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
/// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
/// <returns>A <typeparamref name="T"/> containing the changes between the working directory and the index.</returns>
public virtual T Compare<T>() where T : class
public virtual T Compare<T>() where T : class, IDiffResult
{
return Compare<T>(DiffModifiers.None);
}
Expand All @@ -379,7 +372,7 @@ public virtual T Compare<T>() where T : class
/// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
/// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
/// <returns>A <typeparamref name="T"/> containing the changes between the working directory and the index.</returns>
public virtual T Compare<T>(IEnumerable<string> paths) where T : class
public virtual T Compare<T>(IEnumerable<string> paths) where T : class, IDiffResult
{
return Compare<T>(DiffModifiers.None, paths);
}
Expand All @@ -396,7 +389,7 @@ public virtual T Compare<T>(IEnumerable<string> paths) where T : class
/// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
/// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
/// <returns>A <typeparamref name="T"/> containing the changes between the working directory and the index.</returns>
public virtual T Compare<T>(IEnumerable<string> paths, bool includeUntracked) where T : class
public virtual T Compare<T>(IEnumerable<string> paths, bool includeUntracked) where T : class, IDiffResult
{
return Compare<T>(includeUntracked ? DiffModifiers.IncludeUntracked : DiffModifiers.None, paths);
}
Expand All @@ -417,7 +410,7 @@ public virtual T Compare<T>(IEnumerable<string> paths, bool includeUntracked) wh
/// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
/// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
/// <returns>A <typeparamref name="T"/> containing the changes between the working directory and the index.</returns>
public virtual T Compare<T>(IEnumerable<string> paths, bool includeUntracked, ExplicitPathsOptions explicitPathsOptions) where T : class
public virtual T Compare<T>(IEnumerable<string> paths, bool includeUntracked, ExplicitPathsOptions explicitPathsOptions) where T : class, IDiffResult
{
return Compare<T>(includeUntracked ? DiffModifiers.IncludeUntracked : DiffModifiers.None, paths, explicitPathsOptions);
}
Expand All @@ -443,7 +436,7 @@ public virtual T Compare<T>(
IEnumerable<string> paths,
bool includeUntracked,
ExplicitPathsOptions explicitPathsOptions,
CompareOptions compareOptions) where T : class
CompareOptions compareOptions) where T : class, IDiffResult
{
return Compare<T>(includeUntracked ? DiffModifiers.IncludeUntracked : DiffModifiers.None, paths, explicitPathsOptions, compareOptions);
}
Expand All @@ -452,19 +445,8 @@ internal virtual T Compare<T>(
DiffModifiers diffOptions,
IEnumerable<string> paths = null,
ExplicitPathsOptions explicitPathsOptions = null,
CompareOptions compareOptions = null) where T : class
CompareOptions compareOptions = null) where T : class, IDiffResult
{
Func<DiffSafeHandle, object> builder;

if (!ChangesBuilders.TryGetValue(typeof(T), out builder))
{
throw new LibGit2SharpException(CultureInfo.InvariantCulture,
"Unexpected type '{0}' passed to Compare. Supported values are either '{1}' or '{2}'.",
typeof(T),
typeof(TreeChanges),
typeof(Patch));
}

var comparer = WorkdirToIndex(repo);

if (explicitPathsOptions != null)
Expand All @@ -479,7 +461,7 @@ internal virtual T Compare<T>(

using (DiffSafeHandle diff = BuildDiffList(null, null, comparer, diffOptions, paths, explicitPathsOptions, compareOptions))
{
return (T)builder(diff);
return BuildDiffResult<T>(diff);
}
}

Expand Down
5 changes: 5 additions & 0 deletions LibGit2Sharp/IDiffResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
namespace LibGit2Sharp
{
public interface IDiffResult
{ }
}
1 change: 1 addition & 0 deletions LibGit2Sharp/LibGit2Sharp.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@
<Compile Include="Core\GitCertificateSsh.cs" />
<Compile Include="Core\GitCertificateSshType.cs" />
<Compile Include="CertificateSsh.cs" />
<Compile Include="IDiffResult.cs" />
</ItemGroup>
<ItemGroup>
<CodeAnalysisDictionary Include="CustomDictionary.xml" />
Expand Down
3 changes: 1 addition & 2 deletions LibGit2Sharp/Patch.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace LibGit2Sharp
/// deleted, modified, ..., then consider using a simpler <see cref="TreeChanges"/>.</para>
/// </summary>
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class Patch : IEnumerable<PatchEntryChanges>
public class Patch : IEnumerable<PatchEntryChanges>, IDiffResult
{
private readonly StringBuilder fullPatchBuilder = new StringBuilder();

Expand All @@ -41,7 +41,6 @@ internal Patch(DiffSafeHandle diff)
AddFileChange(delta);
Proxy.git_patch_print(patch, PrintCallBack);
}

}
}

Expand Down
2 changes: 1 addition & 1 deletion LibGit2Sharp/PatchStats.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace LibGit2Sharp
/// <para>The individual patches for each file can be accessed through the indexer of this class.</para>
/// </summary>
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class PatchStats : IEnumerable<ContentChangeStats>
public class PatchStats : IEnumerable<ContentChangeStats>, IDiffResult
{
private readonly IDictionary<FilePath, ContentChangeStats> changes = new Dictionary<FilePath, ContentChangeStats>();
private readonly int totalLinesAdded;
Expand Down
3 changes: 1 addition & 2 deletions LibGit2Sharp/TreeChanges.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace LibGit2Sharp
/// <para>To obtain the actual patch of the diff, use the <see cref="Patch"/> class when calling Compare.</para>.
/// </summary>
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class TreeChanges : IEnumerable<TreeEntryChanges>
public class TreeChanges : IEnumerable<TreeEntryChanges>, IDiffResult
{
private readonly List<TreeEntryChanges> changes = new List<TreeEntryChanges>();
private readonly List<TreeEntryChanges> added = new List<TreeEntryChanges>();
Expand Down Expand Up @@ -91,7 +91,6 @@ IEnumerator IEnumerable.GetEnumerator()

#endregion


/// <summary>
/// List of <see cref="TreeEntryChanges"/> that have been been added.
/// </summary>
Expand Down