From baab84836ab8ed0ebd6a57dae991607b0c24039b Mon Sep 17 00:00:00 2001 From: Tobias Pfeifer Date: Mon, 5 Aug 2013 12:54:23 +0200 Subject: [PATCH] Parse commitId and shortMessage of a commented out line in rebase todo A rebase todo file consists of regular non-comment action lines and comment lines. In case that a regular action line has been commented out (i.e. prefixed with a hash '#'), the RebaseTodoLine that is representing this line should hold the values for commitId and shortMessage even though it's a comment line. This allows to switch between comment and non-comment easily even after the file has been persisted and reread. Change-Id: I56ec2ba08eaf3772e2d74d937dd496209a744d4b Signed-off-by: Tobias Pfeifer Signed-off-by: Matthias Sohn --- .../eclipse/jgit/api/RebaseCommandTest.java | 119 +++++++++++++++ .../org/eclipse/jgit/lib/RebaseTodoFile.java | 139 ++++++++++++------ 2 files changed, 214 insertions(+), 44 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 c1d2b2213..b9f5bc95e 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 @@ -63,11 +63,13 @@ import org.eclipse.jgit.api.MergeResult.MergeStatus; import org.eclipse.jgit.api.RebaseCommand.InteractiveHandler; import org.eclipse.jgit.api.RebaseCommand.Operation; import org.eclipse.jgit.api.RebaseResult.Status; +import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.api.errors.RefNotFoundException; import org.eclipse.jgit.api.errors.UnmergedPathsException; import org.eclipse.jgit.api.errors.WrongRepositoryStateException; import org.eclipse.jgit.dircache.DirCacheCheckout; import org.eclipse.jgit.junit.RepositoryTestCase; +import org.eclipse.jgit.lib.AbbreviatedObjectId; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; @@ -1745,6 +1747,123 @@ public class RebaseCommandTest extends RepositoryTestCase { assertEquals(Action.REWORD, steps.get(1).getAction()); } + @Test + public void testRebaseShouldTryToParseValidLineMarkedAsComment() + throws IOException { + String todo = "# pick 1111111 Valid line commented out with space\n" + + "#edit 2222222 Valid line commented out without space\n" + + "# pick invalidLine Comment line at end\n"; + write(getTodoFile(), todo); + + List steps = db.readRebaseTodo(GIT_REBASE_TODO, true); + assertEquals(3, steps.size()); + + RebaseTodoLine firstLine = steps.get(0); + + assertEquals("1111111", firstLine.getCommit().name()); + assertEquals("Valid line commented out with space", + firstLine.getShortMessage()); + assertEquals("comment", firstLine.getAction().toToken()); + + try { + firstLine.setAction(Action.PICK); + assertEquals("1111111", firstLine.getCommit().name()); + assertEquals("pick", firstLine.getAction().toToken()); + } catch (Exception e) { + fail("Valid parsable RebaseTodoLine that has been commented out should allow to change the action, but failed"); + } + + assertEquals("2222222", steps.get(1).getCommit().name()); + assertEquals("comment", steps.get(1).getAction().toToken()); + + assertEquals(null, steps.get(2).getCommit()); + assertEquals(null, steps.get(2).getShortMessage()); + assertEquals("comment", steps.get(2).getAction().toToken()); + assertEquals("# pick invalidLine Comment line at end", steps.get(2) + .getComment()); + try { + steps.get(2).setAction(Action.PICK); + fail("A comment RebaseTodoLine that doesn't contain a valid parsable line should fail, but doesn't"); + } catch (Exception e) { + // expected + } + + } + + @SuppressWarnings("unused") + @Test + public void testRebaseTodoLineSetComment() throws Exception { + try { + new RebaseTodoLine("This is a invalid comment"); + fail("Constructing a comment line with invalid comment string should fail, but doesn't"); + } catch (JGitInternalException e) { + // expected + } + RebaseTodoLine validCommentLine = new RebaseTodoLine( + "# This is a comment"); + assertEquals(Action.COMMENT, validCommentLine.getAction()); + assertEquals("# This is a comment", validCommentLine.getComment()); + + RebaseTodoLine actionLineToBeChanged = new RebaseTodoLine(Action.EDIT, + AbbreviatedObjectId.fromString("1111111"), "short Message"); + assertEquals(null, actionLineToBeChanged.getComment()); + + try { + actionLineToBeChanged.setComment("invalid comment"); + fail("Setting a invalid comment string should fail but doesn't"); + } catch (JGitInternalException e) { + assertEquals(null, actionLineToBeChanged.getComment()); + } + + actionLineToBeChanged.setComment("# valid comment"); + assertEquals("# valid comment", actionLineToBeChanged.getComment()); + try { + actionLineToBeChanged.setComment("invalid comment"); + fail("Setting a invalid comment string should fail but doesn't"); + } catch (JGitInternalException e) { + // expected + // setting comment failed, but was successfully set before, + // therefore it may not be altered since then + assertEquals("# valid comment", actionLineToBeChanged.getComment()); + } + try { + actionLineToBeChanged.setComment("# line1 \n line2"); + actionLineToBeChanged.setComment("line1 \n line2"); + actionLineToBeChanged.setComment("\n"); + actionLineToBeChanged.setComment("# line1 \r line2"); + actionLineToBeChanged.setComment("line1 \r line2"); + actionLineToBeChanged.setComment("\r"); + actionLineToBeChanged.setComment("# line1 \n\r line2"); + actionLineToBeChanged.setComment("line1 \n\r line2"); + actionLineToBeChanged.setComment("\n\r"); + fail("Setting a multiline comment string should fail but doesn't"); + } catch (JGitInternalException e) { + // expected + } + // Try setting valid comments + actionLineToBeChanged.setComment("# valid comment"); + assertEquals("# valid comment", actionLineToBeChanged.getComment()); + + actionLineToBeChanged.setComment("# \t \t valid comment"); + assertEquals("# \t \t valid comment", + actionLineToBeChanged.getComment()); + + actionLineToBeChanged.setComment("# "); + assertEquals("# ", actionLineToBeChanged.getComment()); + + actionLineToBeChanged.setComment(""); + assertEquals("", actionLineToBeChanged.getComment()); + + actionLineToBeChanged.setComment(" "); + assertEquals(" ", actionLineToBeChanged.getComment()); + + actionLineToBeChanged.setComment("\t\t"); + assertEquals("\t\t", actionLineToBeChanged.getComment()); + + actionLineToBeChanged.setComment(null); + assertEquals(null, actionLineToBeChanged.getComment()); + } + @Test public void testRebaseInteractiveReword() throws Exception { // create file1 on master diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java index a386fff0e..8db75662a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java @@ -51,6 +51,7 @@ import java.io.OutputStreamWriter; import java.util.LinkedList; import java.util.List; +import org.eclipse.jgit.lib.RebaseTodoLine.Action; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; @@ -88,8 +89,6 @@ public class RebaseTodoFile { int tokenBegin = 0; List r = new LinkedList(); while (ptr < buf.length) { - RebaseTodoLine.Action action = null; - AbbreviatedObjectId commit = null; tokenBegin = ptr; ptr = RawParseUtils.nextLF(buf, ptr); int lineStart = tokenBegin; @@ -99,54 +98,106 @@ public class RebaseTodoFile { // Handle comments if (buf[tokenBegin] == '#') { if (includeComments) - r.add(new RebaseTodoLine(RawParseUtils.decode(buf, - tokenBegin, lineEnd + 1))); - continue; - } - // skip leading spaces, tabs, CR - while (tokenBegin <= lineEnd - && (buf[tokenBegin] == ' ' || buf[tokenBegin] == '\t' || buf[tokenBegin] == '\r')) - tokenBegin++; - // Handle empty lines (maybe empty after skipping leading - // whitespace) - if (tokenBegin > lineEnd) { - if (includeComments) - r.add(new RebaseTodoLine(RawParseUtils.decode(buf, - lineStart, 1 + lineEnd))); - continue; - } - int nextSpace = RawParseUtils.next(buf, tokenBegin, ' '); - int tokenCount = 0; - while (tokenCount < 3 && nextSpace < ptr) { - switch (tokenCount) { - case 0: - String actionToken = new String(buf, tokenBegin, nextSpace - - tokenBegin - 1); - tokenBegin = nextSpace; - action = RebaseTodoLine.Action.parse(actionToken); - break; - case 1: - if (action == null) - break; - nextSpace = RawParseUtils.next(buf, tokenBegin, ' '); - String commitToken = new String(buf, tokenBegin, nextSpace - - tokenBegin - 1); - tokenBegin = nextSpace; - commit = AbbreviatedObjectId.fromString(commitToken); - break; - case 2: - if (action == null) - break; - r.add(new RebaseTodoLine(action, commit, RawParseUtils - .decode(buf, tokenBegin, 1 + lineEnd))); - break; + parseComments(buf, tokenBegin, r, lineEnd); + } else { + // skip leading spaces+tabs+cr + tokenBegin = nextParsableToken(buf, tokenBegin, lineEnd); + // Handle empty lines (maybe empty after skipping leading + // whitespace) + if (tokenBegin == -1) { + if (includeComments) + r.add(new RebaseTodoLine(RawParseUtils.decode(buf, + lineStart, 1 + lineEnd))); + continue; } - tokenCount++; + RebaseTodoLine line = parseLine(buf, tokenBegin, lineEnd); + if (line == null) + continue; + r.add(line); } } return r; } + private static void parseComments(byte[] buf, int tokenBegin, + List r, int lineEnd) { + RebaseTodoLine line = null; + String commentString = RawParseUtils.decode(buf, + tokenBegin, lineEnd + 1); + try { + int skip = tokenBegin + 1; // skip '#' + skip = nextParsableToken(buf, skip, lineEnd); + if (skip != -1) { + // try to parse the line as non-comment + line = parseLine(buf, skip, lineEnd); + // successfully parsed as non-comment line + // mark this line as a comment explicitly + line.setAction(Action.COMMENT); + // use the read line as comment string + line.setComment(commentString); + } + } catch (Exception e) { + // parsing as non-comment line failed + line = null; + } finally { + if (line == null) + line = new RebaseTodoLine(commentString); + r.add(line); + } + } + + /** + * Skip leading space, tab, CR and LF characters + * + * @param buf + * @param tokenBegin + * @param lineEnd + * @return the token within the range of the given {@code buf} that doesn't + * need to be skipped, {@code -1} if no such token found within the + * range (i.e. empty line) + */ + private static int nextParsableToken(byte[] buf, int tokenBegin, int lineEnd) { + while (tokenBegin <= lineEnd + && (buf[tokenBegin] == ' ' || buf[tokenBegin] == '\t' || buf[tokenBegin] == '\r')) + tokenBegin++; + if (tokenBegin > lineEnd) + return -1; + return tokenBegin; + } + + private static RebaseTodoLine parseLine(byte[] buf, int tokenBegin, + int lineEnd) { + RebaseTodoLine.Action action = null; + AbbreviatedObjectId commit = null; + + int nextSpace = RawParseUtils.next(buf, tokenBegin, ' '); + int tokenCount = 0; + while (tokenCount < 3 && nextSpace < lineEnd) { + switch (tokenCount) { + case 0: + String actionToken = new String(buf, tokenBegin, nextSpace + - tokenBegin - 1); + tokenBegin = nextSpace; + action = RebaseTodoLine.Action.parse(actionToken); + if (action == null) + return null; // parsing failed + break; + case 1: + nextSpace = RawParseUtils.next(buf, tokenBegin, ' '); + String commitToken = new String(buf, tokenBegin, nextSpace + - tokenBegin - 1); + tokenBegin = nextSpace; + commit = AbbreviatedObjectId.fromString(commitToken); + break; + case 2: + return new RebaseTodoLine(action, commit, RawParseUtils.decode( + buf, tokenBegin, 1 + lineEnd)); + } + tokenCount++; + } + return null; + } + /** * Write a file formatted like a git-rebase-todo file. *