Skip to content

FileHistory Feature #963

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.

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
Apr 14, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
396 changes: 396 additions & 0 deletions LibGit2Sharp.Tests/FileHistoryFixture.cs

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
<Compile Include="CheckoutFixture.cs" />
<Compile Include="CherryPickFixture.cs" />
<Compile Include="DescribeFixture.cs" />
<Compile Include="FileHistoryFixture.cs" />
<Compile Include="GlobalSettingsFixture.cs" />
<Compile Include="PatchStatsFixture.cs" />
<Compile Include="RefSpecFixture.cs" />
Expand Down Expand Up @@ -157,4 +158,4 @@
<Target Name="AfterBuild">
</Target>
-->
</Project>
</Project>
Copy link
Member

Choose a reason for hiding this comment

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

Are we going to give in to the fact that VS removes the trailing new line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What can you do about that in .csproj files?

Copy link
Member

Choose a reason for hiding this comment

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

@ThomasBarnekow It's usually recommended to have all text files ending with a final newline, but VS keeps on removing them from .csproj. One way to work around this is through partial staging. One would only stage the interesting chunks and reset the removal of the newline in the workdir. But don't bother about this, @jamill's right. This is useless ceremony. Let's keep the file as it is.

Are we going to give in to the fact that VS removes the trailing new line?

@jamill I'm surrendering to VisualStudio trimming frenzy 😉

27 changes: 26 additions & 1 deletion LibGit2Sharp/CommitLog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,32 @@ public ICommitLog QueryBy(CommitFilter filter)
return new CommitLog(repo, filter);
}

/// <summary>
/// Returns the list of commits of the repository representing the history of a file beyond renames.
/// </summary>
/// <param name="path">The file's path.</param>
/// <returns>A list of file history entries, ready to be enumerated.</returns>
public IEnumerable<LogEntry> QueryBy(string path)
{
Ensure.ArgumentNotNull(path, "path");

return new FileHistory(repo, path);
}

/// <summary>
/// Returns the list of commits of the repository representing the history of a file beyond renames.
/// </summary>
/// <param name="path">The file's path.</param>
/// <param name="filter">The options used to control which commits will be returned.</param>
/// <returns>A list of file history entries, ready to be enumerated.</returns>
public IEnumerable<LogEntry> QueryBy(string path, FollowFilter filter)
{
Ensure.ArgumentNotNull(path, "path");
Ensure.ArgumentNotNull(filter, "filter");

return new FileHistory(repo, path, new CommitFilter {SortBy = filter.SortBy});
}

/// <summary>
/// Find the best possible merge base given two <see cref="Commit"/>s.
/// </summary>
Expand Down Expand Up @@ -206,7 +232,6 @@ private void FirstParentOnly(bool firstParent)
}
}
}

}

/// <summary>
Expand Down
172 changes: 172 additions & 0 deletions LibGit2Sharp/Core/FileHistory.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;

namespace LibGit2Sharp.Core
{
/// <summary>
/// Represents a file-related log of commits beyond renames.
/// </summary>
internal class FileHistory : IEnumerable<LogEntry>
{
#region Fields

/// <summary>
/// The allowed commit sort strategies.
/// </summary>
private static readonly List<CommitSortStrategies> AllowedSortStrategies = new List<CommitSortStrategies>
{
CommitSortStrategies.Topological,
CommitSortStrategies.Time,
CommitSortStrategies.Topological | CommitSortStrategies.Time
};

/// <summary>
/// The repository.
/// </summary>
private readonly Repository _repo;

/// <summary>
/// The file's path relative to the repository's root.
/// </summary>
private readonly string _path;

/// <summary>
/// The filter to be used in querying the commit log.
/// </summary>
private readonly CommitFilter _queryFilter;

#endregion

#region Constructors

/// <summary>
/// Initializes a new instance of the <see cref="FileHistory"/> class.
/// The commits will be enumerated in reverse chronological order.
/// </summary>
/// <param name="repo">The repository.</param>
/// <param name="path">The file's path relative to the repository's root.</param>
/// <exception cref="ArgumentNullException">If any of the parameters is null.</exception>
internal FileHistory(Repository repo, string path)
: this(repo, path, new CommitFilter())
{ }

/// <summary>
/// Initializes a new instance of the <see cref="FileHistory"/> class.
/// The given <see cref="CommitFilter"/> instance specifies the commit
/// sort strategies and range of commits to be considered.
/// Only the time (corresponding to <code>--date-order</code>) and topological
/// (coresponding to <code>--topo-order</code>) sort strategies are supported.
/// </summary>
/// <param name="repo">The repository.</param>
/// <param name="path">The file's path relative to the repository's root.</param>
/// <param name="queryFilter">The filter to be used in querying the commit log.</param>
/// <exception cref="ArgumentNullException">If any of the parameters is null.</exception>
/// <exception cref="ArgumentException">When an unsupported commit sort strategy is specified.</exception>
internal FileHistory(Repository repo, string path, CommitFilter queryFilter)
{
Ensure.ArgumentNotNull(repo, "repo");
Ensure.ArgumentNotNull(path, "path");
Ensure.ArgumentNotNull(queryFilter, "queryFilter");

// Ensure the commit sort strategy makes sense.
if (!AllowedSortStrategies.Contains(queryFilter.SortBy))
throw new ArgumentException(
"Unsupported sort strategy. Only 'Topological', 'Time', or 'Topological | Time' are allowed.",
"queryFilter");

_repo = repo;
_path = path;
_queryFilter = queryFilter;
}

#endregion

#region IEnumerable<LogEntry> Members

/// <summary>
/// Gets the <see cref="IEnumerator{LogEntry}"/> that enumerates the
/// <see cref="LogEntry"/> instances representing the file's history,
/// including renames (as in <code>git log --follow</code>).
/// </summary>
/// <returns>A <see cref="IEnumerator{LogEntry}"/>.</returns>
public IEnumerator<LogEntry> GetEnumerator()
{
return FullHistory(_repo, _path, _queryFilter).GetEnumerator();
}

IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}

#endregion

/// <summary>
/// Gets the relevant commits in which the given file was created, changed, or renamed.
/// </summary>
/// <param name="repo">The repository.</param>
/// <param name="path">The file's path relative to the repository's root.</param>
/// <param name="filter">The filter to be used in querying the commits log.</param>
/// <returns>A collection of <see cref="LogEntry"/> instances.</returns>
private static IEnumerable<LogEntry> FullHistory(IRepository repo, string path, CommitFilter filter)
{
var map = new Dictionary<Commit, string>();

foreach (var currentCommit in repo.Commits.QueryBy(filter))
{
var currentPath = map.Keys.Count > 0 ? map[currentCommit] : path;
var currentTreeEntry = currentCommit.Tree[currentPath];

if (currentTreeEntry == null)
{
yield break;
}

var parentCount = currentCommit.Parents.Count();
if (parentCount == 0)
{
yield return new LogEntry { Path = currentPath, Commit = currentCommit };
}
else
{
DetermineParentPaths(repo, currentCommit, currentPath, map);

if (parentCount != 1)
{
continue;
}

var parentCommit = currentCommit.Parents.Single();
var parentPath = map[parentCommit];
var parentTreeEntry = parentCommit.Tree[parentPath];

if (parentTreeEntry == null ||
parentTreeEntry.Target.Id != currentTreeEntry.Target.Id ||
parentPath != currentPath)
{
yield return new LogEntry { Path = currentPath, Commit = currentCommit };
}
}
}
}

private static void DetermineParentPaths(IRepository repo, Commit currentCommit, string currentPath, IDictionary<Commit, string> map)
{
foreach (var parentCommit in currentCommit.Parents.Where(parentCommit => !map.ContainsKey(parentCommit)))
{
map.Add(parentCommit, ParentPath(repo, currentCommit, currentPath, parentCommit));
}
}

private static string ParentPath(IRepository repo, Commit currentCommit, string currentPath, Commit parentCommit)
{
var treeChanges = repo.Diff.Compare<TreeChanges>(parentCommit.Tree, currentCommit.Tree);
var treeEntryChanges = treeChanges.FirstOrDefault(c => c.Path == currentPath);
return treeEntryChanges != null && treeEntryChanges.Status == ChangeKind.Renamed
? treeEntryChanges.OldPath
: currentPath;
}
}
}
57 changes: 57 additions & 0 deletions LibGit2Sharp/FollowFilter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
using System;
using System.Collections.Generic;

namespace LibGit2Sharp
{
/// <summary>
/// Criteria used to order the commits of the repository when querying its history.
Copy link
Member

Choose a reason for hiding this comment

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

As we don't expose Since yet, that may be worth adding something like

<para>
  The commits will be enumerated from the current HEAD of the repository.
</para/>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add.

/// <para>
/// The commits will be enumerated from the current HEAD of the repository.
/// </para>
/// </summary>
public sealed class FollowFilter
{
private static readonly List<CommitSortStrategies> AllowedSortStrategies = new List<CommitSortStrategies>
{
CommitSortStrategies.Topological,
CommitSortStrategies.Time,
CommitSortStrategies.Topological | CommitSortStrategies.Time
};

private CommitSortStrategies _sortBy;

/// <summary>
/// Initializes a new instance of <see cref="FollowFilter" />.
/// </summary>
public FollowFilter()
{
SortBy = CommitSortStrategies.Time;
}

/// <summary>
/// The ordering strategy to use.
/// <para>
/// By default, the commits are shown in reverse chronological order.
/// </para>
/// <para>
/// Only 'Topological', 'Time', or 'Topological | Time' are allowed.
Copy link
Member

Choose a reason for hiding this comment

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

👍 Nice touch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

/// </para>
/// </summary>
public CommitSortStrategies SortBy
{
get { return _sortBy; }

set
{
if (!AllowedSortStrategies.Contains(value))
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd rather see this test moved in the QueryBy() method. It should be less surprising to see the method throw rather than a setter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that would mean that you could create an invalid FollowFilter instance and QueryBy() would have to know when the filter is invalid. Wouldn't it be better to keep that knowledge encapsulated in the FollowFilter?

If we moved the test, I wouldn't see the need for FollowFilter anymore. We should then also change QueryBy(string, FollowFilter) into QueryBy(string, CommitSortStrategies).

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And by the way, I would be with you if we talked about using CommitFilter or any other class that is more generic than FollowFilter. But the sole purpose of FollowFilter, from my perspective, is to represent a consistent and valid filter for the purposes of following a file's history.

Copy link
Member

Choose a reason for hiding this comment

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

Hmmm. It looks like my memory was wrong. The Framework design guidelines doesn't specify anything about avoid throwing from setters. Let's keep it this way, then.

Sorry for the noise.

FWIW, the sole purpose of FollowFilter is to get this PR merged ASAP without requiring any additional test coverage. We'll be able to add new properties to FollowFilter as time goes by (along with covering tests). It may eventually evolve into a CommitFilter. When that happens, we'll obsolete the method accepting a FollowFilter and create a new one accepting a CommitFilter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, no problem.

{
throw new ArgumentException(
"Unsupported sort strategy. Only 'Topological', 'Time', or 'Topological | Time' are allowed.",
"value");
}

_sortBy = value;
}
}
}
}
15 changes: 15 additions & 0 deletions LibGit2Sharp/IQueryableCommitLog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,21 @@ public interface IQueryableCommitLog : ICommitLog
/// <returns>A list of commits, ready to be enumerated.</returns>
ICommitLog QueryBy(CommitFilter filter);

/// <summary>
/// Returns the list of commits of the repository representing the history of a file beyond renames.
/// </summary>
/// <param name="path">The file's path.</param>
/// <returns>A list of file history entries, ready to be enumerated.</returns>
IEnumerable<LogEntry> QueryBy(string path);

/// <summary>
/// Returns the list of commits of the repository representing the history of a file beyond renames.
/// </summary>
/// <param name="path">The file's path.</param>
/// <param name="filter">The options used to control which commits will be returned.</param>
/// <returns>A list of file history entries, ready to be enumerated.</returns>
IEnumerable<LogEntry> QueryBy(string path, FollowFilter filter);

/// <summary>
/// Find the best possible merge base given two <see cref="Commit"/>s.
/// </summary>
Expand Down
3 changes: 3 additions & 0 deletions LibGit2Sharp/LibGit2Sharp.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
<Compile Include="CommitOptions.cs" />
<Compile Include="CommitSortStrategies.cs" />
<Compile Include="CompareOptions.cs" />
<Compile Include="Core\FileHistory.cs" />
<Compile Include="Core\Platform.cs" />
<Compile Include="Core\Handles\ConflictIteratorSafeHandle.cs" />
<Compile Include="DescribeOptions.cs" />
Expand All @@ -80,6 +81,8 @@
<Compile Include="Core\Handles\IndexReucEntrySafeHandle.cs" />
<Compile Include="EntryExistsException.cs" />
<Compile Include="FetchOptionsBase.cs" />
<Compile Include="LogEntry.cs" />
<Compile Include="FollowFilter.cs" />
<Compile Include="IBelongToARepository.cs" />
<Compile Include="Identity.cs" />
<Compile Include="IndexNameEntryCollection.cs" />
Expand Down
18 changes: 18 additions & 0 deletions LibGit2Sharp/LogEntry.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
namespace LibGit2Sharp
{
/// <summary>
/// An entry in a file's commit history.
/// </summary>
public sealed class LogEntry
{
/// <summary>
/// The file's path relative to the repository's root.
/// </summary>
public string Path { get; internal set; }

/// <summary>
/// The commit in which the file was created or changed.
/// </summary>
public Commit Commit { get; internal set; }
}
}