From edd0cd4deb361a3e50ef809d5da8a096c9d681fd Mon Sep 17 00:00:00 2001 From: David Alpert Date: Sun, 26 Aug 2012 00:50:32 -0500 Subject: [PATCH] refactoring the Rcheckin command to lean on the GenericCaretaker via a CheckinOptionsHelper when processing special actions in commit messages to associate and resolve work items --- GitTfs/Commands/Rcheckin.cs | 79 +++++++---------- GitTfs/GitTfs.csproj | 1 + GitTfs/Util/CheckinOptionsHelper.cs | 87 ++++++++++++++++++ GitTfsTest/GitTfsTest.csproj | 1 + GitTfsTest/Util/CheckinOptionsHelperTests.cs | 92 ++++++++++++++++++++ 5 files changed, 213 insertions(+), 47 deletions(-) create mode 100644 GitTfs/Util/CheckinOptionsHelper.cs create mode 100644 GitTfsTest/Util/CheckinOptionsHelperTests.cs diff --git a/GitTfs/Commands/Rcheckin.cs b/GitTfs/Commands/Rcheckin.cs index 9a4490f8..4e7823bd 100644 --- a/GitTfs/Commands/Rcheckin.cs +++ b/GitTfs/Commands/Rcheckin.cs @@ -7,6 +7,7 @@ using NDesk.Options; using Sep.Git.Tfs.Core; using StructureMap; using System.Text.RegularExpressions; +using Sep.Git.Tfs.Util; namespace Sep.Git.Tfs.Commands { @@ -16,6 +17,7 @@ namespace Sep.Git.Tfs.Commands { private readonly TextWriter _stdout; private readonly CheckinOptions _checkinOptions; + private readonly CheckinOptionsHelper _checkinOptionsHelper; private readonly TfsWriter _writer; private bool Quick { get; set; } @@ -24,6 +26,7 @@ namespace Sep.Git.Tfs.Commands { _stdout = stdout; _checkinOptions = checkinOptions; + _checkinOptionsHelper = new CheckinOptionsHelper(_stdout); _writer = writer; } @@ -45,30 +48,6 @@ namespace Sep.Git.Tfs.Commands return _writer.Write("HEAD", PerformRCheckin); } - private String ProcessWorkItems(string commitMessage) - { - MatchCollection workitemMatches; - if ((workitemMatches = Sep.Git.Tfs.GitTfsConstants.TfsWorkItemRegex.Matches(commitMessage)).Count > 0) - { - foreach (Match match in workitemMatches) - { - switch (match.Groups["action"].Value) - { - case "associate": - _stdout.WriteLine("Associating with work item {0}", match.Groups["item_id"]); - _checkinOptions.WorkItemsToAssociate.Add(match.Groups["item_id"].Value); - break; - case "resolve": - _stdout.WriteLine("Resolving work item {0}", match.Groups["item_id"]); - _checkinOptions.WorkItemsToResolve.Add(match.Groups["item_id"].Value); - break; - } - } - return Sep.Git.Tfs.GitTfsConstants.TfsWorkItemRegex.Replace(commitMessage, "").Trim(' ', '\r', '\n'); - } - return commitMessage; - } - private int PerformRCheckin(TfsChangesetInfo parentChangeset) { var tfsRemote = parentChangeset.Remote; @@ -109,19 +88,19 @@ namespace Sep.Git.Tfs.Commands string target = strs[0]; string[] gitParents = strs.AsEnumerable().Skip(1).Where(hash => hash != currentParent).ToArray(); - string commitMessage = ProcessWorkItems(repo.GetCommitMessage(target, currentParent).Trim(' ', '\r', '\n')); - _stdout.WriteLine("Starting checkin of {0} '{1}'", target.Substring(0, 8), commitMessage); - _checkinOptions.CheckinComment = commitMessage; - long newChangesetId = tfsRemote.Checkin(target, currentParent, parentChangeset); - tfsRemote.FetchWithMerge(newChangesetId, gitParents); - if (tfsRemote.MaxChangesetId != newChangesetId) - throw new GitTfsException("error: New TFS changesets were found. Rcheckin was not finished."); + string commitMessage = repo.GetCommitMessage(target, currentParent).Trim(' ', '\r', '\n'); + using (var caretaker = _checkinOptionsHelper.UpdateCheckinOptionsForThisCommit(_checkinOptions, commitMessage)) + { + _stdout.WriteLine("Starting checkin of {0} '{1}'", target.Substring(0, 8), _checkinOptions.CheckinComment); + long newChangesetId = tfsRemote.Checkin(target, currentParent, parentChangeset); + tfsRemote.FetchWithMerge(newChangesetId, gitParents); + if (tfsRemote.MaxChangesetId != newChangesetId) + throw new GitTfsException("error: New TFS changesets were found. Rcheckin was not finished."); - currentParent = target; - parentChangeset = new TfsChangesetInfo { ChangesetId = newChangesetId, GitCommit = tfsRemote.MaxCommitHash, Remote = tfsRemote }; - _stdout.WriteLine("Done with {0}.", target); - _checkinOptions.WorkItemsToAssociate.Clear(); - _checkinOptions.WorkItemsToResolve.Clear(); + currentParent = target; + parentChangeset = new TfsChangesetInfo { ChangesetId = newChangesetId, GitCommit = tfsRemote.MaxCommitHash, Remote = tfsRemote }; + _stdout.WriteLine("Done with {0}.", target); + } } _stdout.WriteLine("No more to rcheckin."); @@ -143,22 +122,28 @@ namespace Sep.Git.Tfs.Commands string target = strs[0]; string[] gitParents = strs.AsEnumerable().Skip(1).Where(hash => hash != tfsLatest).ToArray(); - string commitMessage = ProcessWorkItems(repo.GetCommitMessage(target, tfsLatest).Trim(' ', '\r', '\n')); - _stdout.WriteLine("Starting checkin of {0} '{1}'", target.Substring(0, 8), commitMessage); - _checkinOptions.CheckinComment = commitMessage; - long newChangesetId = tfsRemote.Checkin(target, parentChangeset); - tfsRemote.FetchWithMerge(newChangesetId, gitParents); - if (tfsRemote.MaxChangesetId != newChangesetId) - throw new GitTfsException("error: New TFS changesets were found. Rcheckin was not finished."); + string commitMessage = repo.GetCommitMessage(target, tfsLatest).Trim(' ', '\r', '\n'); + using (var caretaker = _checkinOptionsHelper.UpdateCheckinOptionsForThisCommit(_checkinOptions, commitMessage)) + { + _stdout.WriteLine("Starting checkin of {0} '{1}'", target.Substring(0, 8), _checkinOptions.CheckinComment); + long newChangesetId = tfsRemote.Checkin(target, parentChangeset); + tfsRemote.FetchWithMerge(newChangesetId, gitParents); + if (tfsRemote.MaxChangesetId != newChangesetId) + throw new GitTfsException("error: New TFS changesets were found. Rcheckin was not finished."); + tfsLatest = tfsRemote.MaxCommitHash; + parentChangeset = new TfsChangesetInfo { ChangesetId = newChangesetId, GitCommit = tfsLatest, Remote = tfsRemote }; + _stdout.WriteLine("Done with {0}, rebasing tail onto new TFS-commit...", target); tfsLatest = tfsRemote.MaxCommitHash; parentChangeset = new TfsChangesetInfo {ChangesetId = newChangesetId, GitCommit = tfsLatest, Remote = tfsRemote}; _stdout.WriteLine("Done with {0}, rebasing tail onto new TFS-commit...", target); + tfsLatest = tfsRemote.MaxCommitHash; + parentChangeset = new TfsChangesetInfo { ChangesetId = newChangesetId, GitCommit = tfsLatest, Remote = tfsRemote }; + _stdout.WriteLine("Done with {0}, rebasing tail onto new TFS-commit...", target); - repo.CommandNoisy("rebase", "--preserve-merges", "--onto", tfsLatest, target); - _stdout.WriteLine("Rebase done successfully."); - _checkinOptions.WorkItemsToAssociate.Clear(); - _checkinOptions.WorkItemsToResolve.Clear(); + repo.CommandNoisy("rebase", "--preserve-merges", "--onto", tfsLatest, target); + _stdout.WriteLine("Rebase done successfully."); + } } } } diff --git a/GitTfs/GitTfs.csproj b/GitTfs/GitTfs.csproj index e1bb4661..7cf90601 100644 --- a/GitTfs/GitTfs.csproj +++ b/GitTfs/GitTfs.csproj @@ -171,6 +171,7 @@ + diff --git a/GitTfs/Util/CheckinOptionsHelper.cs b/GitTfs/Util/CheckinOptionsHelper.cs new file mode 100644 index 00000000..ad35c381 --- /dev/null +++ b/GitTfs/Util/CheckinOptionsHelper.cs @@ -0,0 +1,87 @@ +using System; +using System.IO; +using Sep.Git.Tfs.Commands; +using System.Text.RegularExpressions; + +namespace Sep.Git.Tfs.Util +{ + /// + /// Stores the state of the before parsing + /// a git commit message for special actions. Packages up restore + /// actions inside of a so that any + /// special actions in the commit message can be temporarily + /// set in the semi-global without + /// affecting state later on. + /// + /// + /// This class extracts the pre-checkin commit message parsing that + /// enables special git-tfs commands: + /// https://github.com/git-tfs/git-tfs/wiki/Special-actions-in-commit-messages + /// + public class CheckinOptionsHelper + { + TextWriter writer; + + public CheckinOptionsHelper(TextWriter writer) + { + this.writer = writer; + } + + public GenericCaretaker UpdateCheckinOptionsForThisCommit(CheckinOptions checkinOptions, string commitMessage) + { + Action restoreCheckinComment = ApplyCommitMessage(checkinOptions, commitMessage); + Action undoWorkItemCommands = ProcessWorkItemCommands(checkinOptions, writer); + + return new GenericCaretaker(() => + { + restoreCheckinComment(); + undoWorkItemCommands(); + }); + } + + private Action ApplyCommitMessage(CheckinOptions checkinOptions, string commitMessage) + { + // store existing state + string originalCheckinComment = checkinOptions.CheckinComment; + + // operate + checkinOptions.CheckinComment = commitMessage; + + // delegate the restoration of state + return () => + { + checkinOptions.CheckinComment = originalCheckinComment; + }; + } + + private Action ProcessWorkItemCommands(CheckinOptions checkinOptions, TextWriter writer) + { + MatchCollection workitemMatches; + if ((workitemMatches = Sep.Git.Tfs.GitTfsConstants.TfsWorkItemRegex.Matches(checkinOptions.CheckinComment)).Count > 0) + { + foreach (Match match in workitemMatches) + { + switch (match.Groups["action"].Value) + { + case "associate": + writer.WriteLine("Associating with work item {0}", match.Groups["item_id"]); + checkinOptions.WorkItemsToAssociate.Add(match.Groups["item_id"].Value); + break; + case "resolve": + writer.WriteLine("Resolving work item {0}", match.Groups["item_id"]); + checkinOptions.WorkItemsToResolve.Add(match.Groups["item_id"].Value); + break; + } + } + checkinOptions.CheckinComment = Sep.Git.Tfs.GitTfsConstants.TfsWorkItemRegex.Replace(checkinOptions.CheckinComment, "").Trim(' ', '\r', '\n'); + } + + // delegate the restoration of state + return () => + { + checkinOptions.WorkItemsToAssociate.Clear(); + checkinOptions.WorkItemsToResolve.Clear(); + }; + } + } +} diff --git a/GitTfsTest/GitTfsTest.csproj b/GitTfsTest/GitTfsTest.csproj index 5f852539..939b17ca 100644 --- a/GitTfsTest/GitTfsTest.csproj +++ b/GitTfsTest/GitTfsTest.csproj @@ -99,6 +99,7 @@ + diff --git a/GitTfsTest/Util/CheckinOptionsHelperTests.cs b/GitTfsTest/Util/CheckinOptionsHelperTests.cs new file mode 100644 index 00000000..16df5f23 --- /dev/null +++ b/GitTfsTest/Util/CheckinOptionsHelperTests.cs @@ -0,0 +1,92 @@ +using System; +using System.IO; +using System.Linq; +using Sep.Git.Tfs.Commands; +using Sep.Git.Tfs.Util; +using Xunit; + +namespace Sep.Git.Tfs.Test.Util +{ + public class CheckinOptionsHelperTests + { + [Fact] + public void Update_preserves_original_commit() + { + TextWriter writer = new StringWriter(); + CheckinOptionsHelper helper = new CheckinOptionsHelper(writer); + + string originalCheckinComment = "command-line input"; + CheckinOptions checkinOptions = new CheckinOptions() + { + CheckinComment = originalCheckinComment + }; + + string commitMessage = +@"test message + + formatted git commit message"; + + string expectedCheckinComment = +@"test message + + formatted git commit message"; + + using (var caretaker = helper.UpdateCheckinOptionsForThisCommit(checkinOptions, commitMessage)) + { + Assert.Equal(expectedCheckinComment, checkinOptions.CheckinComment); + } + + Assert.Equal(originalCheckinComment, checkinOptions.CheckinComment); + } + + [Fact] + public void Update_associates_and_clears_work_items() + { + StringWriter textWriter = new StringWriter(); + CheckinOptionsHelper helper = new CheckinOptionsHelper(textWriter); + + CheckinOptions checkinOptions = new CheckinOptions(); + + string commitMessage = +@"test message + + formatted git commit message + + git-tfs-work-item: 1234 associate"; + + using (var caretaker = helper.UpdateCheckinOptionsForThisCommit(checkinOptions, commitMessage)) + { + Assert.Equal(1, checkinOptions.WorkItemsToAssociate.Count); + Assert.Equal("1234", checkinOptions.WorkItemsToAssociate.First()); + } + + Assert.Equal(0, checkinOptions.WorkItemsToAssociate.Count); + Assert.Equal("Associating with work item 1234" + textWriter.NewLine, textWriter.ToString()); + } + + [Fact] + public void Update_resolves_and_clears_work_items() + { + StringWriter textWriter = new StringWriter(); + CheckinOptionsHelper helper = new CheckinOptionsHelper(textWriter); + + CheckinOptions checkinOptions = new CheckinOptions(); + + string commitMessage = +@"test message + + formatted git commit message + + git-tfs-work-item: 1234 resolve"; + + using (var caretaker = helper.UpdateCheckinOptionsForThisCommit(checkinOptions, commitMessage)) + { + Assert.Equal(1, checkinOptions.WorkItemsToResolve.Count); + Assert.Equal("1234", checkinOptions.WorkItemsToResolve.First()); + } + + Assert.Equal(0, checkinOptions.WorkItemsToResolve.Count); + Assert.Equal("Resolving work item 1234" + textWriter.NewLine, textWriter.ToString()); + } + } +} \ No newline at end of file