diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java index 64475f5d5..917b6c329 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java @@ -36,6 +36,7 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.RepositoryState; import org.eclipse.jgit.lib.Sets; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.merge.ContentMergeStrategy; import org.eclipse.jgit.merge.MergeStrategy; import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason; @@ -2018,6 +2019,73 @@ public void testMergeConflictWithMessageOption() throws Exception { } } + @Test + public void testMergeConflictWithMessageAndCommentChar() throws Exception { + try (Git git = new Git(db)) { + writeTrashFile("a", "1\na\n3\n"); + git.add().addFilepattern("a").call(); + RevCommit initialCommit = git.commit().setMessage("initial").call(); + + createBranch(initialCommit, "refs/heads/side"); + checkoutBranch("refs/heads/side"); + + writeTrashFile("a", "1\na(side)\n3\n"); + git.add().addFilepattern("a").call(); + git.commit().setMessage("side").call(); + + checkoutBranch("refs/heads/master"); + + writeTrashFile("a", "1\na(main)\n3\n"); + git.add().addFilepattern("a").call(); + git.commit().setMessage("main").call(); + + StoredConfig config = db.getConfig(); + config.setString("core", null, "commentChar", "^"); + + Ref sideBranch = db.exactRef("refs/heads/side"); + + git.merge().include(sideBranch).setStrategy(MergeStrategy.RESOLVE) + .setMessage("user message").call(); + + assertEquals("user message\n\n^ Conflicts:\n^\ta\n", + db.readMergeCommitMsg()); + } + } + + @Test + public void testMergeConflictWithMessageAndCommentCharAuto() + throws Exception { + try (Git git = new Git(db)) { + writeTrashFile("a", "1\na\n3\n"); + git.add().addFilepattern("a").call(); + RevCommit initialCommit = git.commit().setMessage("initial").call(); + + createBranch(initialCommit, "refs/heads/side"); + checkoutBranch("refs/heads/side"); + + writeTrashFile("a", "1\na(side)\n3\n"); + git.add().addFilepattern("a").call(); + git.commit().setMessage("side").call(); + + checkoutBranch("refs/heads/master"); + + writeTrashFile("a", "1\na(main)\n3\n"); + git.add().addFilepattern("a").call(); + git.commit().setMessage("main").call(); + + StoredConfig config = db.getConfig(); + config.setString("core", null, "commentChar", "auto"); + + Ref sideBranch = db.exactRef("refs/heads/side"); + + git.merge().include(sideBranch).setStrategy(MergeStrategy.RESOLVE) + .setMessage("#user message").call(); + + assertEquals("#user message\n\n; Conflicts:\n;\ta\n", + db.readMergeCommitMsg()); + } + } + private static void setExecutable(Git git, String path, boolean executable) { FS.DETECTED.setExecute( new File(git.getRepository().getWorkTree(), path), executable); 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 c64ff0b1c..d574e45f6 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 @@ -30,6 +30,7 @@ import org.eclipse.jgit.api.MergeResult.MergeStatus; import org.eclipse.jgit.api.RebaseCommand.InteractiveHandler; +import org.eclipse.jgit.api.RebaseCommand.InteractiveHandler2; import org.eclipse.jgit.api.RebaseCommand.Operation; import org.eclipse.jgit.api.RebaseResult.Status; import org.eclipse.jgit.api.errors.InvalidRebaseStepException; @@ -46,6 +47,7 @@ import org.eclipse.jgit.events.ListenerHandle; import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.lib.AbbreviatedObjectId; +import org.eclipse.jgit.lib.CommitConfig.CleanupMode; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; @@ -56,6 +58,7 @@ import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.ReflogEntry; import org.eclipse.jgit.lib.RepositoryState; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.merge.MergeStrategy; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevSort; @@ -3410,6 +3413,99 @@ public String modifyCommitMessage(String commit) { } + @Test + public void testInteractiveRebaseSquashFixupSequence() throws Exception { + // create file1, add and commit + writeTrashFile(FILE1, "file1"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("commit1").call(); + + // modify file1, add and commit + writeTrashFile(FILE1, "modified file1"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("commit2").call(); + + // modify file1, add and commit + writeTrashFile(FILE1, "modified file1 a second time"); + git.add().addFilepattern(FILE1).call(); + // Make it difficult; use git standard comment characters in the commit + // messages + git.commit().setMessage("#commit3").call(); + + // modify file1, add and commit + writeTrashFile(FILE1, "modified file1 a third time"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("@commit4").call(); + + // modify file1, add and commit + writeTrashFile(FILE1, "modified file1 a fourth time"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage(";commit5").call(); + + StoredConfig config = git.getRepository().getConfig(); + config.setString("core", null, "commentChar", "auto"); + // With "auto", we should end up with '@' being used as comment + // character (commit4 is skipped, so it should not advance the + // character). + RebaseResult result = git.rebase().setUpstream("HEAD~4") + .runInteractively(new InteractiveHandler2() { + + @Override + public void prepareSteps(List steps) { + try { + steps.get(0).setAction(Action.PICK); + steps.get(1).setAction(Action.SQUASH); + steps.get(2).setAction(Action.FIXUP); + steps.get(3).setAction(Action.SQUASH); + } catch (IllegalTodoFileModification e) { + fail("unexpected exception: " + e); + } + } + + @Override + public String modifyCommitMessage(String commit) { + fail("should not be called"); + return commit; + } + + @Override + public ModifyResult editCommitMessage(String message, + CleanupMode mode, char commentChar) { + assertEquals('@', commentChar); + assertEquals("@ This is a combination of 4 commits.\n" + + "@ The first commit's message is:\n" + + "commit2\n" + + "@ This is the 2nd commit message:\n" + + "#commit3\n" + + "@ The 3rd commit message will be skipped:\n" + + "@ @commit4\n" + + "@ This is the 4th commit message:\n" + + ";commit5", message); + return new ModifyResult() { + + @Override + public String getMessage() { + return message; + } + + @Override + public CleanupMode getCleanupMode() { + return mode; + } + + @Override + public boolean shouldAddChangeId() { + return false; + } + }; + } + }).call(); + assertEquals(Status.OK, result.getStatus()); + Iterator logIterator = git.log().all().call().iterator(); + String actualCommitMsg = logIterator.next().getFullMessage(); + assertEquals("commit2\n#commit3\n;commit5", actualCommitMsg); + } + private File getTodoFile() { File todoFile = new File(db.getDirectory(), GIT_REBASE_TODO); return todoFile; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java index f88179ac1..ceba89d16 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java @@ -30,6 +30,7 @@ import org.eclipse.jgit.events.WorkingTreeModifiedEvent; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.CommitConfig; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; @@ -183,9 +184,13 @@ public CherryPickResult call() throws GitAPIException, NoMessageException, String message; if (unmergedPaths != null) { + CommitConfig cfg = repo.getConfig() + .get(CommitConfig.KEY); + message = srcCommit.getFullMessage(); + char commentChar = cfg.getCommentChar(message); message = new MergeMessageFormatter() - .formatWithConflicts(srcCommit.getFullMessage(), - unmergedPaths, '#'); + .formatWithConflicts(message, unmergedPaths, + commentChar); } else { message = srcCommit.getFullMessage(); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java index 7a591aa3b..d0a794789 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java @@ -233,11 +233,25 @@ public RevCommit call() throws GitAPIException, AbortedByHookException, config = repo.getConfig().get(CommitConfig.KEY); cleanupMode = config.resolve(cleanupMode, cleanDefaultIsStrip); } - char comments; - if (commentChar == null) { - comments = '#'; // TODO use git config core.commentChar - } else { - comments = commentChar.charValue(); + char comments = (char) 0; + if (CleanupMode.STRIP.equals(cleanupMode) + || CleanupMode.SCISSORS.equals(cleanupMode)) { + if (commentChar == null) { + if (config == null) { + config = repo.getConfig().get(CommitConfig.KEY); + } + if (config.isAutoCommentChar()) { + // We're supposed to pick a character that isn't used, + // but then cleaning up won't remove any lines. So don't + // bother. + comments = (char) 0; + cleanupMode = CleanupMode.WHITESPACE; + } else { + comments = config.getCommentChar(); + } + } else { + comments = commentChar.charValue(); + } } message = CommitConfig.cleanText(message, cleanupMode, comments); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java index ce068b630..ed4a5342b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java @@ -34,6 +34,7 @@ import org.eclipse.jgit.events.WorkingTreeModifiedEvent; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.CommitConfig; import org.eclipse.jgit.lib.Config.ConfigEnum; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.NullProgressMonitor; @@ -404,8 +405,11 @@ public MergeResult call() throws GitAPIException, NoHeadException, MergeStatus.FAILED, mergeStrategy, lowLevelResults, failingPaths, null); } + CommitConfig cfg = repo.getConfig().get(CommitConfig.KEY); + char commentChar = cfg.getCommentChar(message); String mergeMessageWithConflicts = new MergeMessageFormatter() - .formatWithConflicts(mergeMessage, unmergedPaths, '#'); + .formatWithConflicts(mergeMessage, unmergedPaths, + commentChar); repo.writeMergeCommitMsg(mergeMessageWithConflicts); return new MergeResult(null, merger.getBaseCommitId(), new ObjectId[] { headCommit.getId(), diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java index 2b0d8ce1c..4e0d9d78c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java @@ -449,7 +449,8 @@ private RebaseResult processStep(RebaseTodoLine step, boolean shouldPick) String oldMessage = commitToPick.getFullMessage(); CleanupMode mode = commitConfig.resolve(CleanupMode.DEFAULT, true); boolean[] doChangeId = { false }; - String newMessage = editCommitMessage(doChangeId, oldMessage, mode); + String newMessage = editCommitMessage(doChangeId, oldMessage, mode, + commitConfig.getCommentChar(oldMessage)); try (Git git = new Git(repo)) { newHead = git.commit() .setMessage(newMessage) @@ -494,12 +495,12 @@ private RebaseResult processStep(RebaseTodoLine step, boolean shouldPick) } private String editCommitMessage(boolean[] doChangeId, String message, - @NonNull CleanupMode mode) { + @NonNull CleanupMode mode, char commentChar) { String newMessage; CommitConfig.CleanupMode cleanup; if (interactiveHandler instanceof InteractiveHandler2) { InteractiveHandler2.ModifyResult modification = ((InteractiveHandler2) interactiveHandler) - .editCommitMessage(message, mode, '#'); + .editCommitMessage(message, mode, commentChar); newMessage = modification.getMessage(); cleanup = modification.getCleanupMode(); if (CleanupMode.DEFAULT.equals(cleanup)) { @@ -511,7 +512,7 @@ private String editCommitMessage(boolean[] doChangeId, String message, cleanup = CommitConfig.CleanupMode.STRIP; doChangeId[0] = false; } - return CommitConfig.cleanText(newMessage, cleanup, '#'); + return CommitConfig.cleanText(newMessage, cleanup, commentChar); } private RebaseResult cherryPickCommit(RevCommit commitToPick) @@ -808,8 +809,9 @@ private RevCommit squashIntoPrevious(boolean sequenceContainsSquash, if (isLast) { boolean[] doChangeId = { false }; if (sequenceContainsSquash) { + char commentChar = commitMessage.charAt(0); commitMessage = editCommitMessage(doChangeId, commitMessage, - CleanupMode.STRIP); + CleanupMode.STRIP, commentChar); } retNewHead = git.commit() .setMessage(commitMessage) @@ -829,30 +831,60 @@ private RevCommit squashIntoPrevious(boolean sequenceContainsSquash, } @SuppressWarnings("nls") - private static String composeSquashMessage(boolean isSquash, + private String composeSquashMessage(boolean isSquash, RevCommit commitToPick, String currSquashMessage, int count) { StringBuilder sb = new StringBuilder(); String ordinal = getOrdinal(count); - sb.setLength(0); - sb.append("# This is a combination of ").append(count) - .append(" commits.\n"); - // Add the previous message without header (i.e first line) - sb.append(currSquashMessage - .substring(currSquashMessage.indexOf('\n') + 1)); - sb.append("\n"); - if (isSquash) { - sb.append("# This is the ").append(count).append(ordinal) - .append(" commit message:\n"); - sb.append(commitToPick.getFullMessage()); + // currSquashMessage is always non-empty here, and the first character + // is the comment character used so far. + char commentChar = currSquashMessage.charAt(0); + String newMessage = commitToPick.getFullMessage(); + if (!isSquash) { + sb.append(commentChar).append(" This is a combination of ") + .append(count).append(" commits.\n"); + // Add the previous message without header (i.e first line) + sb.append(currSquashMessage + .substring(currSquashMessage.indexOf('\n') + 1)); + sb.append('\n'); + sb.append(commentChar).append(" The ").append(count).append(ordinal) + .append(" commit message will be skipped:\n") + .append(commentChar).append(' '); + sb.append(newMessage.replaceAll("([\n\r])", + "$1" + commentChar + ' ')); } else { - sb.append("# The ").append(count).append(ordinal) - .append(" commit message will be skipped:\n# "); - sb.append(commitToPick.getFullMessage().replaceAll("([\n\r])", - "$1# ")); + String currentMessage = currSquashMessage; + if (commitConfig.isAutoCommentChar()) { + // Figure out a new comment character taking into account the + // new message + String cleaned = CommitConfig.cleanText(currentMessage, + CommitConfig.CleanupMode.STRIP, commentChar) + '\n' + + newMessage; + char newCommentChar = commitConfig.getCommentChar(cleaned); + if (newCommentChar != commentChar) { + currentMessage = replaceCommentChar(currentMessage, + commentChar, newCommentChar); + commentChar = newCommentChar; + } + } + sb.append(commentChar).append(" This is a combination of ") + .append(count).append(" commits.\n"); + // Add the previous message without header (i.e first line) + sb.append( + currentMessage.substring(currentMessage.indexOf('\n') + 1)); + sb.append('\n'); + sb.append(commentChar).append(" This is the ").append(count) + .append(ordinal).append(" commit message:\n"); + sb.append(newMessage); } return sb.toString(); } + private String replaceCommentChar(String message, char oldChar, + char newChar) { + // (?m) - Switch on multi-line matching; \h - horizontal whitespace + return message.replaceAll("(?m)^(\\h*)" + oldChar, "$1" + newChar); //$NON-NLS-1$ //$NON-NLS-2$ + } + private static String getOrdinal(int count) { switch (count % 10) { case 1: @@ -886,10 +918,11 @@ static int parseSquashFixupSequenceCount(String currSquashMessage) { private void initializeSquashFixupFile(String messageFile, String fullMessage) throws IOException { - rebaseState - .createFile( - messageFile, - "# This is a combination of 1 commits.\n# The first commit's message is:\n" + fullMessage); //$NON-NLS-1$); + char commentChar = commitConfig.getCommentChar(fullMessage); + rebaseState.createFile(messageFile, + commentChar + " This is a combination of 1 commits.\n" //$NON-NLS-1$ + + commentChar + " The first commit's message is:\n" //$NON-NLS-1$ + + fullMessage); } private String getOurCommitName() { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java index db88ad8dc..513f579b6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java @@ -30,6 +30,7 @@ import org.eclipse.jgit.events.WorkingTreeModifiedEvent; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.CommitConfig; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; @@ -185,9 +186,12 @@ public RevCommit call() throws NoMessageException, UnmergedPathsException, MergeStatus.CONFLICTING, strategy, merger.getMergeResults(), failingPaths, null); if (!merger.failed() && !unmergedPaths.isEmpty()) { + CommitConfig config = repo.getConfig() + .get(CommitConfig.KEY); + char commentChar = config.getCommentChar(newMessage); String message = new MergeMessageFormatter() .formatWithConflicts(newMessage, - merger.getUnmergedPaths(), '#'); + merger.getUnmergedPaths(), commentChar); repo.writeRevertHead(srcCommit.getId()); repo.writeMergeCommitMsg(message); }