Use git config core.commentChar

This concerns committing, creating merge conflict messages and creating
and editing squash messages. In a squash message, once the comment
character has been determined initially is always the first character.
Note that if core.commentChar=auto and there is a sequence of squashes,
it may be necessary to change the comment character when a new message
is added.

Bug: 579325
Change-Id: Idca19284a0240cd322e7512ea299a03658e1b2c1
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

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);

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<RebaseTodoLine> 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<RevCommit> 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;

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(),
+								.formatWithConflicts(message, unmergedPaths,
-										unmergedPaths, '#');
+										commentChar);
 					} else {
 						message = srcCommit.getFullMessage();
 					}

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;
+			char comments = (char) 0;
-			if (commentChar == null) {
+			if (CleanupMode.STRIP.equals(cleanupMode)
-				comments = '#'; // TODO use git config core.commentChar
+					|| CleanupMode.SCISSORS.equals(cleanupMode)) {
-			} else {
+				if (commentChar == null) {
-				comments = commentChar.charValue();
+					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);
 

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(),

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);
+		// currSquashMessage is always non-empty here, and the first character
-		sb.append("# This is a combination of ").append(count)
+		// is the comment character used so far.
-				.append(" commits.\n");
+		char commentChar = currSquashMessage.charAt(0);
-		// Add the previous message without header (i.e first line)
+		String newMessage = commitToPick.getFullMessage();
-		sb.append(currSquashMessage
+		if (!isSquash) {
-				.substring(currSquashMessage.indexOf('\n') + 1));
+			sb.append(commentChar).append(" This is a combination of ")
-		sb.append("\n");
+					.append(count).append(" commits.\n");
-		if (isSquash) {
+			// Add the previous message without header (i.e first line)
-			sb.append("# This is the ").append(count).append(ordinal)
+			sb.append(currSquashMessage
-					.append(" commit message:\n");
+					.substring(currSquashMessage.indexOf('\n') + 1));
-			sb.append(commitToPick.getFullMessage());
+			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)
+			String currentMessage = currSquashMessage;
-					.append(" commit message will be skipped:\n# ");
+			if (commitConfig.isAutoCommentChar()) {
-			sb.append(commitToPick.getFullMessage().replaceAll("([\n\r])",
+				// Figure out a new comment character taking into account the
-					"$1# "));
+				// 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
+		char commentChar = commitConfig.getCommentChar(fullMessage);
-				.createFile(
+		rebaseState.createFile(messageFile,
-						messageFile,
+				commentChar + " This is a combination of 1 commits.\n" //$NON-NLS-1$
-						"# This is a combination of 1 commits.\n# The first commit's message is:\n" + fullMessage); //$NON-NLS-1$);
+						+ commentChar + " The first commit's message is:\n" //$NON-NLS-1$
+						+ fullMessage);
 	}
 
 	private String getOurCommitName() {

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);
 					}