Enhance reading of git-rebase-todo formatted files

Reading and writing files formatted like the git-rebase-todo files was
hidden in the RebaseCommand. Certain constructs (like leading tabs and
spaces) have not been handled as in native git. Also the upcoming
rebase interactive feature in EGit needs reading/writing these files
independently from a RebaseCommand.

Therefore reading and writing those files has been moved to the
Repository class. RebaseCommand gets smaller because of that and doesn't
have to deal with reading/writing files.

Additional tests for empty todo-list files, or files containing comments
have been added.

Change-Id: I323f3619952fecdf28ddf50139a88e0bea34f5ba
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Also-by: Tobias Pfeifer <to.pfeifer@sap.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

1 files changed, 122 insertions, 25 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
index ef5a1eaa0c..c1d2b22136 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
@@ -60,10 +60,8 @@ import java.util.Iterator;
 import java.util.List;
 
 import org.eclipse.jgit.api.MergeResult.MergeStatus;
-import org.eclipse.jgit.api.RebaseCommand.Action;
 import org.eclipse.jgit.api.RebaseCommand.InteractiveHandler;
 import org.eclipse.jgit.api.RebaseCommand.Operation;
-import org.eclipse.jgit.api.RebaseCommand.Step;
 import org.eclipse.jgit.api.RebaseResult.Status;
 import org.eclipse.jgit.api.errors.RefNotFoundException;
 import org.eclipse.jgit.api.errors.UnmergedPathsException;
@@ -73,6 +71,8 @@ import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.RebaseTodoLine;
+import org.eclipse.jgit.lib.RebaseTodoLine.Action;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.ReflogEntry;
 import org.eclipse.jgit.lib.RepositoryState;
@@ -85,6 +85,8 @@ import org.junit.Before;
 import org.junit.Test;
 
 public class RebaseCommandTest extends RepositoryTestCase {
+	private static final String GIT_REBASE_TODO = "rebase-merge/git-rebase-todo";
+
 	private static final String FILE1 = "file1";
 
 	protected Git git;
@@ -1497,15 +1499,18 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		try {
 			String line = br.readLine();
 			while (line != null) {
-				String actionToken = line.substring(0, line.indexOf(' '));
-				Action action = null;
-				try {
-					action = Action.parse(actionToken);
-				} catch (Exception e) {
-					// ignore
+				int firstBlank = line.indexOf(' ');
+				if (firstBlank != -1) {
+					String actionToken = line.substring(0, firstBlank);
+					Action action = null;
+					try {
+						action = Action.parse(actionToken);
+					} catch (Exception e) {
+						// ignore
+					}
+					if (Action.PICK.equals(action))
+						count++;
 				}
-				if (action != null)
-					count++;
 				line = br.readLine();
 			}
 			return count;
@@ -1625,11 +1630,61 @@ public class RebaseCommandTest extends RepositoryTestCase {
 				+ "# Comment line at end\n";
 		write(getTodoFile(), todo);
 
-		RebaseCommand rebaseCommand = git.rebase();
-		List<Step> steps = rebaseCommand.loadSteps();
+		List<RebaseTodoLine> steps = db.readRebaseTodo(GIT_REBASE_TODO, false);
 		assertEquals(2, steps.size());
-		assertEquals("1111111", steps.get(0).commit.name());
-		assertEquals("2222222", steps.get(1).commit.name());
+		assertEquals("1111111", steps.get(0).getCommit().name());
+		assertEquals("2222222", steps.get(1).getCommit().name());
+	}
+
+	@Test
+	public void testRebaseShouldNotFailIfUserAddCommentLinesInPrepareSteps()
+			throws Exception {
+		commitFile(FILE1, FILE1, "master");
+		RevCommit c2 = commitFile("file2", "file2", "master");
+
+		// update files on master
+		commitFile(FILE1, "blah", "master");
+		RevCommit c4 = commitFile("file2", "more change", "master");
+
+		RebaseResult res = git.rebase().setUpstream("HEAD~2")
+				.runInteractively(new InteractiveHandler() {
+					public void prepareSteps(List<RebaseTodoLine> steps) {
+						steps.add(0, new RebaseTodoLine(
+								"# Comment that should not be processed"));
+					}
+
+					public String modifyCommitMessage(String commit) {
+						fail("modifyCommitMessage() was not expected to be called");
+						return commit;
+					}
+				}).call();
+
+		assertEquals(RebaseResult.Status.FAST_FORWARD, res.getStatus());
+
+		RebaseResult res2 = git.rebase().setUpstream("HEAD~2")
+				.runInteractively(new InteractiveHandler() {
+					public void prepareSteps(List<RebaseTodoLine> steps) {
+						steps.get(0).setAction(Action.COMMENT); // delete
+																// RevCommit c4
+					}
+
+					public String modifyCommitMessage(String commit) {
+						fail("modifyCommitMessage() was not expected to be called");
+						return commit;
+					}
+				}).call();
+
+		assertEquals(RebaseResult.Status.OK, res2.getStatus());
+
+		ObjectId headId = db.resolve(Constants.HEAD);
+		RevWalk rw = new RevWalk(db);
+		RevCommit rc = rw.parseCommit(headId);
+
+		ObjectId head1Id = db.resolve(Constants.HEAD + "~1");
+		RevCommit rc1 = rw.parseCommit(head1Id);
+
+		assertEquals(rc.getFullMessage(), c4.getFullMessage());
+		assertEquals(rc1.getFullMessage(), c2.getFullMessage());
 	}
 
 	@Test
@@ -1638,13 +1693,56 @@ public class RebaseCommandTest extends RepositoryTestCase {
 				+ "reword 2222222 Commit 2\n";
 		write(getTodoFile(), todo);
 
-		RebaseCommand rebaseCommand = git.rebase();
-		List<Step> steps = rebaseCommand.loadSteps();
+		List<RebaseTodoLine> steps = db.readRebaseTodo(GIT_REBASE_TODO, false);
+
+		assertEquals(2, steps.size());
+		assertEquals("1111111", steps.get(0).getCommit().name());
+		assertEquals("2222222", steps.get(1).getCommit().name());
+		assertEquals(Action.REWORD, steps.get(1).getAction());
+	}
+
+	@Test
+	public void testEmptyRebaseTodo() throws Exception {
+		write(getTodoFile(), "");
+		assertEquals(0, db.readRebaseTodo(GIT_REBASE_TODO, true).size());
+		assertEquals(0, db.readRebaseTodo(GIT_REBASE_TODO, false).size());
+	}
+
+	@Test
+	public void testOnlyCommentRebaseTodo() throws Exception {
+		write(getTodoFile(), "# a b c d e\n# e f");
+		assertEquals(0, db.readRebaseTodo(GIT_REBASE_TODO, false).size());
+		List<RebaseTodoLine> lines = db.readRebaseTodo(GIT_REBASE_TODO, true);
+		assertEquals(2, lines.size());
+		for (RebaseTodoLine line : lines)
+			assertEquals(Action.COMMENT, line.getAction());
+		write(getTodoFile(), "# a b c d e\n# e f\n");
+		assertEquals(0, db.readRebaseTodo(GIT_REBASE_TODO, false).size());
+		lines = db.readRebaseTodo(GIT_REBASE_TODO, true);
+		assertEquals(2, lines.size());
+		for (RebaseTodoLine line : lines)
+			assertEquals(Action.COMMENT, line.getAction());
+		write(getTodoFile(), " 	 \r\n# a b c d e\r\n# e f\r\n#");
+		assertEquals(0, db.readRebaseTodo(GIT_REBASE_TODO, false).size());
+		lines = db.readRebaseTodo(GIT_REBASE_TODO, true);
+		assertEquals(4, lines.size());
+		for (RebaseTodoLine line : lines)
+			assertEquals(Action.COMMENT, line.getAction());
+	}
+
+	@Test
+	public void testLeadingSpacesRebaseTodo() throws Exception {
+		String todo =	"  \t\t pick 1111111 Commit 1\n"
+					+ "\t\n"
+					+ "\treword 2222222 Commit 2\n";
+		write(getTodoFile(), todo);
+
+		List<RebaseTodoLine> steps = db.readRebaseTodo(GIT_REBASE_TODO, false);
 
 		assertEquals(2, steps.size());
-		assertEquals("1111111", steps.get(0).commit.name());
-		assertEquals("2222222", steps.get(1).commit.name());
-		assertEquals(Action.REWORD, steps.get(1).action);
+		assertEquals("1111111", steps.get(0).getCommit().name());
+		assertEquals("2222222", steps.get(1).getCommit().name());
+		assertEquals(Action.REWORD, steps.get(1).getAction());
 	}
 
 	@Test
@@ -1672,8 +1770,8 @@ public class RebaseCommandTest extends RepositoryTestCase {
 
 		RebaseResult res = git.rebase().setUpstream("HEAD~2")
 				.runInteractively(new InteractiveHandler() {
-					public void prepareSteps(List<Step> steps) {
-						steps.get(0).action = Action.REWORD;
+					public void prepareSteps(List<RebaseTodoLine> steps) {
+						steps.get(0).setAction(Action.REWORD);
 					}
 					public String modifyCommitMessage(String commit) {
 						return "rewritten commit message";
@@ -1713,8 +1811,8 @@ public class RebaseCommandTest extends RepositoryTestCase {
 
 		RebaseResult res = git.rebase().setUpstream("HEAD~2")
 				.runInteractively(new InteractiveHandler() {
-					public void prepareSteps(List<Step> steps) {
-						steps.get(0).action = Action.EDIT;
+					public void prepareSteps(List<RebaseTodoLine> steps) {
+						steps.get(0).setAction(Action.EDIT);
 					}
 
 					public String modifyCommitMessage(String commit) {
@@ -1741,8 +1839,7 @@ public class RebaseCommandTest extends RepositoryTestCase {
 	}
 
 	private File getTodoFile() {
-		File todoFile = new File(db.getDirectory(),
-				"rebase-merge/git-rebase-todo");
+		File todoFile = new File(db.getDirectory(), GIT_REBASE_TODO);
 		return todoFile;
 	}
 }