From acf737c3d105f533aa056eba3428c026504c3459 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Tue, 15 Aug 2017 08:54:40 +0200 Subject: [PATCH] Improve commit message validation & remove leading whitespace Before committing, EGit checked whether the commit message was empty. However, if a commit message only contained a Change-Id line, it was considered not empty and would pass. Strengthen the check to consider commit messages containing only footer lines to be empty. Also remove leading whitespace (including leading empty lines) before actually committing. Change-Id: I6ad287c7bf105f6515f23be18f240047fbd99410 Signed-off-by: Thomas Wolf --- .../org/eclipse/egit/core/op/CommitOperation.java | 20 ++++++++++++++------ .../ui/internal/dialogs/CommitMessageComponent.java | 13 ++++++++++++- .../egit/ui/internal/dialogs/CreateTagDialog.java | 10 ++++++++++ .../internal/dialogs/SpellcheckableMessageArea.java | 2 +- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/op/CommitOperation.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/op/CommitOperation.java index 86fb272d1..a75345bb9 100644 --- a/org.eclipse.egit.core/src/org/eclipse/egit/core/op/CommitOperation.java +++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/op/CommitOperation.java @@ -19,6 +19,7 @@ import java.util.Collection; import java.util.Date; import java.util.HashSet; import java.util.TimeZone; +import java.util.regex.Pattern; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IWorkspace; @@ -53,15 +54,18 @@ import org.eclipse.team.core.TeamException; */ public class CommitOperation implements IEGitOperation { + private static final Pattern LEADING_WHITESPACE = Pattern + .compile("^[\\h\\v]+"); //$NON-NLS-1$ + Collection commitFileList; private boolean commitWorkingDirChanges = false; - private String author; + private final String author; - private String committer; + private final String committer; - private String message; + private final String message; private boolean amending = false; @@ -94,7 +98,7 @@ public class CommitOperation implements IEGitOperation { String author, String committer, String message) throws CoreException { this.author = author; this.committer = committer; - this.message = message; + this.message = stripLeadingWhitespace(message); if (filesToCommit != null && filesToCommit.length > 0) setRepository(filesToCommit[0]); if (filesToCommit != null) @@ -122,7 +126,7 @@ public class CommitOperation implements IEGitOperation { this.repo = repository; this.author = author; this.committer = committer; - this.message = message; + this.message = stripLeadingWhitespace(message); if (filesToCommit != null) commitFileList = new HashSet(filesToCommit); if (notTracked != null) @@ -142,10 +146,14 @@ public class CommitOperation implements IEGitOperation { this.repo = repository; this.author = author; this.committer = committer; - this.message = message; + this.message = stripLeadingWhitespace(message); this.commitIndex = true; } + private String stripLeadingWhitespace(String text) { + return text == null ? "" //$NON-NLS-1$ + : LEADING_WHITESPACE.matcher(text).replaceFirst(""); //$NON-NLS-1$ + } private void setRepository(IFile file) throws CoreException { RepositoryMapping mapping = RepositoryMapping.getMapping(file); diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitMessageComponent.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitMessageComponent.java index 8ad6b9415..d85ff5b37 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitMessageComponent.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitMessageComponent.java @@ -21,6 +21,7 @@ package org.eclipse.egit.ui.internal.dialogs; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.regex.Pattern; import org.eclipse.egit.core.RevUtils; import org.eclipse.egit.core.internal.gerrit.GerritUtil; @@ -29,6 +30,7 @@ import org.eclipse.egit.ui.CommitMessageWithCaretPosition; import org.eclipse.egit.ui.UIPreferences; import org.eclipse.egit.ui.UIUtils; import org.eclipse.egit.ui.UIUtils.IPreviousValueProposalHandler; +import org.eclipse.egit.ui.internal.CommonUtils; import org.eclipse.egit.ui.internal.UIText; import org.eclipse.egit.ui.internal.commit.CommitHelper; import org.eclipse.egit.ui.internal.commit.CommitHelper.CommitInfo; @@ -69,6 +71,9 @@ import org.eclipse.ui.PlatformUI; */ public class CommitMessageComponent { + private static final Pattern ANY_NON_WHITESPACE = Pattern + .compile("[^\\h\\v]"); //$NON-NLS-1$ + /** * Status provider for whether a commit operation should be enabled or not */ @@ -493,7 +498,13 @@ public class CommitMessageComponent { public boolean checkCommitInfo() { updateStateFromUI(); - if (commitMessage.trim().length() == 0) { + String text = getCommitMessage(); + // Strip footers + int footer = CommonUtils.getFooterOffset(text); + if (footer >= 0) { + text = text.substring(0, footer); + } + if (!ANY_NON_WHITESPACE.matcher(text).find()) { MessageDialog.openWarning(getShell(), UIText.CommitDialog_ErrorNoMessage, UIText.CommitDialog_ErrorMustEnterCommitMessage); diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CreateTagDialog.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CreateTagDialog.java index defa074ec..671edff81 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CreateTagDialog.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CreateTagDialog.java @@ -98,6 +98,12 @@ public class CreateTagDialog extends TitleAreaDialog { */ private static final int CREATE_AND_START_PUSH_ID = 23; + /** + * We trim leading whitespace from the tag commit message. + */ + private static final Pattern LEADING_WHITESPACE = Pattern + .compile("^[\\h\\v]+"); //$NON-NLS-1$ + private String tagName; private String tagMessage; @@ -382,6 +388,10 @@ public class CreateTagDialog extends TitleAreaDialog { if (commitCombo != null) tagCommit = commitCombo.getValue(); tagMessage = tagMessageText.getCommitMessage(); + if (tagMessage != null) { + tagMessage = LEADING_WHITESPACE.matcher(tagMessage) + .replaceFirst(""); //$NON-NLS-1$ + } overwriteTag = overwriteButton.getSelection(); okPressed(); } else { diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/SpellcheckableMessageArea.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/SpellcheckableMessageArea.java index 721238b2c..b0e110119 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/SpellcheckableMessageArea.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/SpellcheckableMessageArea.java @@ -113,7 +113,7 @@ public class SpellcheckableMessageArea extends Composite { private static final Pattern TRAILING_WHITE_SPACE_ON_LINES = Pattern .compile("\\h+$", Pattern.MULTILINE); //$NON-NLS-1$ - private static final Pattern TRAILING_NEWLINES = Pattern.compile("\\n+$"); //$NON-NLS-1$ + private static final Pattern TRAILING_NEWLINES = Pattern.compile("\\v+$"); //$NON-NLS-1$ private static class TextViewerAction extends Action implements IUpdate { -- 2.11.4.GIT