aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Wolf2018-06-11 07:40:15 -0400
committerThomas Wolf2018-07-05 12:56:51 -0400
commitf25e7c92b13fc6517da3d78315489a19b57328d7 (patch)
treebb82831da55c13368dc794b77bb505914e4393e0
parenta8f3b251792dc203433db89300c744c55c8ab4b0 (diff)
downloadegit-f25e7c92b13fc6517da3d78315489a19b57328d7.zip
egit-f25e7c92b13fc6517da3d78315489a19b57328d7.tar.gz
egit-f25e7c92b13fc6517da3d78315489a19b57328d7.tar.xz
Staging view: update author/committer on config changes
When the config changes, update the committer (and possibly also a signed-off line in the commit message) to the possibly new user.name and user.email values. If the author is the same as the committer, also update the author of the commit. Correct the logic in CommitMessageComponent for updating the sign-off settings. Previously, editing the committer field would switch off the "sign off" flag if the committer was invalid (quite common if you type a new value), and thus the user had to re-enable it manually and correct an old or wrong signed-off-by line explicitly. Bug: 533019 Change-Id: I13252da7beec6cd1ecd1ad2831b4d85c94d46636 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
-rw-r--r--org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/StagingViewTester.java10
-rw-r--r--org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/stagview/StagingViewTest.java228
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitMessageComponent.java80
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java38
4 files changed, 331 insertions, 25 deletions
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/StagingViewTester.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/StagingViewTester.java
index a16b7d1..232f7c7 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/StagingViewTester.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/StagingViewTester.java
@@ -59,6 +59,16 @@ public class StagingViewTester {
.setText(committer);
}
+ public String getAuthor() {
+ return stagingView.bot().textWithLabel(UIText.StagingView_Author)
+ .getText();
+ }
+
+ public String getCommitter() {
+ return stagingView.bot().textWithLabel(UIText.StagingView_Committer)
+ .getText();
+ }
+
public void setCommitMessage(String message) {
stagingView.bot().styledTextWithLabel(UIText.StagingView_CommitMessage)
.setText(message);
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/stagview/StagingViewTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/stagview/StagingViewTest.java
index fdc602a..9c92877 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/stagview/StagingViewTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/stagview/StagingViewTest.java
@@ -13,6 +13,7 @@ package org.eclipse.egit.ui.test.stagview;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -20,9 +21,13 @@ import org.eclipse.egit.ui.common.StagingViewTester;
import org.eclipse.egit.ui.test.CommitMessageUtil;
import org.eclipse.egit.ui.test.TestUtil;
import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.lib.ConfigConstants;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RepositoryState;
+import org.eclipse.jgit.lib.StoredConfig;
import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.util.RawParseUtils;
import org.junit.Test;
public class StagingViewTest extends AbstractStagingViewTestCase {
@@ -102,14 +107,22 @@ public class StagingViewTest extends AbstractStagingViewTestCase {
.getShortMessage());
}
- private void commitOneFileChange(String fileContent) throws Exception {
+ private StagingViewTester commitOneFileChange(String fileContent)
+ throws Exception {
+ return commitOneFileChange(fileContent, TestUtil.TESTAUTHOR,
+ TestUtil.TESTCOMMITTER);
+ }
+
+ private StagingViewTester commitOneFileChange(String fileContent,
+ String author,
+ String committer) throws Exception {
setContent(fileContent);
StagingViewTester stagingViewTester = StagingViewTester
.openStagingView();
stagingViewTester.stageFile(FILE1_PATH);
- stagingViewTester.setAuthor(TestUtil.TESTAUTHOR);
- stagingViewTester.setCommitter(TestUtil.TESTCOMMITTER);
+ stagingViewTester.setAuthor(author);
+ stagingViewTester.setCommitter(committer);
stagingViewTester.setCommitMessage("Commit message");
stagingViewTester.setInsertChangeId(true);
stagingViewTester.setSignedOff(true);
@@ -117,5 +130,214 @@ public class StagingViewTest extends AbstractStagingViewTestCase {
assertTrue(commitMessage.indexOf("Change-Id") > 0);
assertTrue(commitMessage.indexOf("Signed-off-by") > 0);
stagingViewTester.commit();
+ return stagingViewTester;
+ }
+
+ @Test
+ public void testCommitMessageCommitterChangeSignOff() throws Exception {
+ setContent("something");
+
+ StagingViewTester stagingViewTester = StagingViewTester
+ .openStagingView();
+ stagingViewTester.setAuthor(TestUtil.TESTAUTHOR);
+ stagingViewTester.setCommitter(TestUtil.TESTCOMMITTER);
+ stagingViewTester.setCommitMessage("Commit message");
+ stagingViewTester.setSignedOff(true);
+ String commitMessage = stagingViewTester.getCommitMessage();
+ assertTrue("Should have a signed-off footer",
+ commitMessage.indexOf("Signed-off-by") > 0);
+ // Edit the committer field: pretend the user typed a backspace,
+ // deleting the closing ">"
+ stagingViewTester.setCommitter(TestUtil.TESTCOMMITTER.substring(0,
+ TestUtil.TESTCOMMITTER.length() - 1));
+ // Now the committer field has an invalid value.
+ assertTrue("Sign off should still be enabled",
+ stagingViewTester.getSignedOff());
+ assertEquals("Commit message should be unchanged", commitMessage,
+ stagingViewTester.getCommitMessage());
+ // Now change it back to some valid value.
+ stagingViewTester.setCommitter("Somebody <some.body@some.where.org>");
+ assertEquals("Commit message should be updated",
+ commitMessage.replace(TestUtil.TESTCOMMITTER,
+ "Somebody <some.body@some.where.org>"),
+ stagingViewTester.getCommitMessage());
+ assertTrue("Sign off should still be enabled",
+ stagingViewTester.getSignedOff());
+ }
+
+ @Test
+ public void testCommitMessageConfigChange() throws Exception {
+ setContent("something");
+
+ StagingViewTester stagingViewTester = StagingViewTester
+ .openStagingView();
+ stagingViewTester.setAuthor(TestUtil.TESTAUTHOR);
+ stagingViewTester.setCommitter(TestUtil.TESTCOMMITTER);
+ stagingViewTester.setCommitMessage("Commit message");
+ stagingViewTester.setSignedOff(true);
+ String commitMessage = stagingViewTester.getCommitMessage();
+ assertTrue("Should have a signed-off footer",
+ commitMessage.indexOf("Signed-off-by") > 0);
+ StoredConfig cfg = repository.getConfig();
+ cfg.load();
+ cfg.setString(ConfigConstants.CONFIG_USER_SECTION, null,
+ ConfigConstants.CONFIG_KEY_NAME, "Some One");
+ cfg.save();
+ TestUtil.processUIEvents();
+ String expectedCommitter = "Some One <" + TestUtil.TESTCOMMITTER_EMAIL
+ + '>';
+ assertEquals("Author should be unchanged", TestUtil.TESTAUTHOR,
+ stagingViewTester.getAuthor());
+ assertEquals("Committer should be changed", expectedCommitter,
+ stagingViewTester.getCommitter());
+ assertEquals("Commit message should be updated",
+ commitMessage.replace(TestUtil.TESTCOMMITTER,
+ expectedCommitter),
+ stagingViewTester.getCommitMessage());
+ assertTrue("Sign-off should be enabled",
+ stagingViewTester.getSignedOff());
+ }
+
+ @Test
+ public void testCommitMessageConfigChangeNoSignOff() throws Exception {
+ setContent("something");
+
+ StagingViewTester stagingViewTester = StagingViewTester
+ .openStagingView();
+ stagingViewTester.setAuthor(TestUtil.TESTAUTHOR);
+ stagingViewTester.setCommitter(TestUtil.TESTCOMMITTER);
+ stagingViewTester.setCommitMessage("Commit message");
+ stagingViewTester.setSignedOff(true);
+ String commitMessage = stagingViewTester.getCommitMessage();
+ assertTrue("Should have a signed-off footer",
+ commitMessage.indexOf("Signed-off-by") > 0);
+ stagingViewTester.setSignedOff(false);
+ commitMessage = stagingViewTester.getCommitMessage();
+ assertTrue("Should not have a signed-off footer",
+ commitMessage.indexOf("Signed-off-by") < 0);
+ StoredConfig cfg = repository.getConfig();
+ cfg.load();
+ cfg.setString(ConfigConstants.CONFIG_USER_SECTION, null,
+ ConfigConstants.CONFIG_KEY_NAME, "Some One");
+ cfg.save();
+ TestUtil.processUIEvents();
+ String expectedCommitter = "Some One <" + TestUtil.TESTCOMMITTER_EMAIL
+ + '>';
+ assertEquals("Author should be unchanged", TestUtil.TESTAUTHOR,
+ stagingViewTester.getAuthor());
+ assertEquals("Committer should be changed", expectedCommitter,
+ stagingViewTester.getCommitter());
+ assertEquals("Commit message should be unchanged", commitMessage,
+ stagingViewTester.getCommitMessage());
+ assertFalse("Sign-off should be disabled",
+ stagingViewTester.getSignedOff());
+ }
+
+ @Test
+ public void testCommitMessageConfigChangeWithAuthor() throws Exception {
+ setContent("something");
+
+ StagingViewTester stagingViewTester = StagingViewTester
+ .openStagingView();
+ stagingViewTester.setAuthor(TestUtil.TESTCOMMITTER);
+ stagingViewTester.setCommitter(TestUtil.TESTCOMMITTER);
+ stagingViewTester.setCommitMessage("Commit message");
+ stagingViewTester.setSignedOff(true);
+ String commitMessage = stagingViewTester.getCommitMessage();
+ assertTrue("Should have a signed-off footer",
+ commitMessage.indexOf("Signed-off-by") > 0);
+ StoredConfig cfg = repository.getConfig();
+ cfg.load();
+ cfg.setString(ConfigConstants.CONFIG_USER_SECTION, null,
+ ConfigConstants.CONFIG_KEY_NAME, "Some One");
+ cfg.save();
+ TestUtil.processUIEvents();
+ String expectedCommitter = "Some One <" + TestUtil.TESTCOMMITTER_EMAIL
+ + '>';
+ assertEquals("Author should be changed", expectedCommitter,
+ stagingViewTester.getAuthor());
+ assertEquals("Committer should be changed", expectedCommitter,
+ stagingViewTester.getCommitter());
+ assertEquals("Commit message should be updated",
+ commitMessage.replace(TestUtil.TESTCOMMITTER,
+ expectedCommitter),
+ stagingViewTester.getCommitMessage());
+ assertTrue("Sign-off should be enabled",
+ stagingViewTester.getSignedOff());
+ }
+
+ @Test
+ public void testCommitMessageConfigChangeBranchSwitchToNew()
+ throws Exception {
+ setContent("something");
+ StagingViewTester stagingViewTester = StagingViewTester
+ .openStagingView();
+ stagingViewTester.setAuthor(TestUtil.TESTCOMMITTER);
+ stagingViewTester.setCommitter(TestUtil.TESTCOMMITTER);
+ stagingViewTester.setCommitMessage("Commit message");
+ stagingViewTester.setSignedOff(true);
+ String commitMessage = stagingViewTester.getCommitMessage();
+ assertTrue("Should have a signed-off footer",
+ commitMessage.indexOf("Signed-off-by") > 0);
+ try (Git git = new Git(repository)) {
+ git.checkout().setAllPaths(true).setCreateBranch(true)
+ .setName("refs/heads/myBranch").call();
+ }
+ TestUtil.joinJobs(
+ org.eclipse.egit.core.JobFamilies.INDEX_DIFF_CACHE_UPDATE);
+ TestUtil.processUIEvents();
+ assertEquals("Author should be unchanged", TestUtil.TESTCOMMITTER,
+ stagingViewTester.getAuthor());
+ assertEquals("Committer should be unchanged", TestUtil.TESTCOMMITTER,
+ stagingViewTester.getCommitter());
+ assertEquals("Commit message should be unchanged", commitMessage,
+ stagingViewTester.getCommitMessage());
+ assertTrue("Sign-off should be enabled",
+ stagingViewTester.getSignedOff());
+
+ }
+
+ @Test
+ public void testCommitMessageConfigChangeAmending() throws Exception {
+ // Make a commit directly -- we want to test amending a commit made by
+ // someone else (for instance, fetched from Gerrit).
+ setContent("something");
+ try (Git git = new Git(repository)) {
+ git.add().addFilepattern(FILE1_PATH).call();
+ PersonIdent author = RawParseUtils
+ .parsePersonIdent(TestUtil.TESTAUTHOR);
+ git.commit().setAuthor(author).setCommitter(author)
+ .setMessage("Author's commit\n\nSigned-off-by: "
+ + TestUtil.TESTAUTHOR)
+ .call();
+ }
+ StagingViewTester stagingViewTester = StagingViewTester
+ .openStagingView();
+ stagingViewTester.setAmend(true);
+ stagingViewTester.setSignedOff(true);
+ String commitMessage = stagingViewTester.getCommitMessage();
+ assertTrue("Commit message should have two signed-off lines",
+ commitMessage.contains(TestUtil.TESTAUTHOR)
+ && commitMessage.contains(TestUtil.TESTCOMMITTER));
+ StoredConfig cfg = repository.getConfig();
+ cfg.load();
+ cfg.setString(ConfigConstants.CONFIG_USER_SECTION, null,
+ ConfigConstants.CONFIG_KEY_NAME, "Some One");
+ cfg.save();
+ TestUtil.processUIEvents();
+ assertEquals("Author should be unchanged", TestUtil.TESTAUTHOR,
+ stagingViewTester.getAuthor());
+ String expectedCommitter = "Some One <" + TestUtil.TESTCOMMITTER_EMAIL
+ + '>';
+ assertEquals("Committer should be changed", expectedCommitter,
+ stagingViewTester.getCommitter());
+ assertTrue("Sign-off should be enabled",
+ stagingViewTester.getSignedOff());
+ assertTrue("Amend should be enabled", stagingViewTester.getAmend());
+ commitMessage = stagingViewTester.getCommitMessage();
+ assertTrue("Commit message should have two signed-off lines",
+ commitMessage.contains(TestUtil.TESTAUTHOR)
+ && commitMessage.contains(expectedCommitter)
+ && !commitMessage.contains(TestUtil.TESTCOMMITTER));
}
}
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 31a3fb8..7afff02 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
@@ -396,10 +396,23 @@ public class CommitMessageComponent {
*
*/
public void updateUIFromState() {
- commitText.setText(commitMessage);
- commitText.getTextWidget().setCaretOffset(caretPosition);
- authorText.setText(author);
+ updateUIFromState(true);
+ }
+
+ /**
+ * Set the UI widgets to the values from the internal state.
+ *
+ * @param withCommitMessage
+ * {@code true} if the commit message shall be updated, too,
+ * {@code false} to only update author and committer
+ */
+ public void updateUIFromState(boolean withCommitMessage) {
+ if (withCommitMessage) {
+ commitText.setText(commitMessage);
+ commitText.getTextWidget().setCaretOffset(caretPosition);
+ }
committerText.setText(committer);
+ authorText.setText(author);
}
/**
@@ -564,20 +577,41 @@ public class CommitMessageComponent {
}
});
committerText.addModifyListener(new ModifyListener() {
- String oldCommitter = committerText.getText();
+ String oldCommitter = committerText.getText().trim();
@Override
public void modifyText(ModifyEvent e) {
- if (!listenersEnabled || !committerText.isEnabled())
+ String newCommitter = committerText.getText().trim();
+ if (!listenersEnabled || !committerText.isEnabled()) {
+ if (!oldCommitter.equals(newCommitter) && RawParseUtils
+ .parsePersonIdent(newCommitter) != null) {
+ oldCommitter = newCommitter;
+ }
return;
- if (signedOff) {
- // the commit message is signed
- // the signature must be updated
- String newCommitter = committerText.getText();
- String oldSignOff = getSignedOff(oldCommitter);
- String newSignOff = getSignedOff(newCommitter);
- commitText.setText(replaceSignOff(commitText.getText(),
- oldSignOff, newSignOff));
+ }
+ if (!oldCommitter.equals(newCommitter) && RawParseUtils
+ .parsePersonIdent(newCommitter) != null) {
+ String oldCommitText = commitText.getText();
+ String newCommitText = oldCommitText;
+ String currentAuthor = authorText.getText().trim();
+ if (newCommitter.equals(currentAuthor)) {
+ if (signedOff) {
+ // Only add a new signed-off if there isn't one
+ // already
+ String signOff = getSignedOff(newCommitter);
+ if (!hasSignOff(oldCommitText, signOff)) {
+ newCommitText = signOff(oldCommitText);
+ }
+ }
+ } else {
+ String oldSignOff = getSignedOff(oldCommitter);
+ String newSignOff = getSignedOff(newCommitter);
+ newCommitText = replaceSignOff(oldCommitText,
+ oldSignOff, newSignOff);
+ }
+ if (!oldCommitText.equals(newCommitText)) {
+ commitText.setText(newCommitText);
+ }
oldCommitter = newCommitter;
}
listener.statusUpdated();
@@ -787,7 +821,7 @@ public class CommitMessageComponent {
}
private String getSignedOff() {
- return getSignedOff(committerText.getText());
+ return getSignedOff(committerText.getText().trim());
}
private String getSignedOff(String signer) {
@@ -824,10 +858,14 @@ public class CommitMessageComponent {
private void updateSignedOffButton() {
String curText = commitText.getText();
- if (!curText.endsWith(Text.DELIMITER))
+ if (!curText.endsWith(Text.DELIMITER)) {
curText += Text.DELIMITER;
- signedOff = curText.indexOf(getSignedOff() + Text.DELIMITER) != -1;
- listener.updateSignedOffToggleSelection(signedOff);
+ }
+ if (RawParseUtils
+ .parsePersonIdent(committerText.getText().trim()) != null) {
+ signedOff = curText.indexOf(getSignedOff() + Text.DELIMITER) != -1;
+ listener.updateSignedOffToggleSelection(signedOff);
+ }
}
private void refreshSignedOffBy() {
@@ -848,6 +886,14 @@ public class CommitMessageComponent {
}
}
+ private boolean hasSignOff(String input, String signature) {
+ String curText = input;
+ if (!curText.endsWith(Text.DELIMITER)) {
+ curText += Text.DELIMITER;
+ }
+ return curText.indexOf(signature + Text.DELIMITER) >= 0;
+ }
+
private String replaceSignOff(String input, String oldSignOff,
String newSignOff) {
assert input != null;
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java
index d491846..0cf0d56 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java
@@ -749,6 +749,8 @@ public class StagingView extends ViewPart
private ListenerHandle refsChangedListener;
+ private ListenerHandle configChangedListener;
+
private LocalResourceManager resources = new LocalResourceManager(
JFaceResources.getResources());
@@ -3625,6 +3627,7 @@ public class StagingView extends ViewPart
*/
private void clearRepository(@Nullable Repository repository) {
saveCommitMessageComponentState();
+ removeRepositoryListeners();
realRepository = repository;
currentRepository = null;
if (isDisposed()) {
@@ -3722,12 +3725,15 @@ public class StagingView extends ViewPart
if (repositoryChanged) {
// Reset paths, they're from the old repository
resetPathsToExpand();
- if (refsChangedListener != null)
- refsChangedListener.remove();
+ removeRepositoryListeners();
refsChangedListener = repository.getListenerList()
.addRefsChangedListener(
event -> updateRebaseButtonVisibility(repository
.getRepositoryState().isRebasing()));
+ configChangedListener = repository.getListenerList()
+ .addConfigChangedListener(
+ event -> updateCommitAuthorAndCommitter(
+ repository));
}
final StagingViewUpdate update = new StagingViewUpdate(repository,
indexDiff, null);
@@ -3808,6 +3814,17 @@ public class StagingView extends ViewPart
});
}
+ private void removeRepositoryListeners() {
+ if (refsChangedListener != null) {
+ refsChangedListener.remove();
+ refsChangedListener = null;
+ }
+ if (configChangedListener != null) {
+ configChangedListener.remove();
+ configChangedListener = null;
+ }
+ }
+
/**
* The max number of changed files we can handle in the "list" presentation
* without freezing Eclipse UI for a too long time.
@@ -4016,6 +4033,19 @@ public class StagingView extends ViewPart
updateMessage();
}
+ private void updateCommitAuthorAndCommitter(Repository repository) {
+ CommitHelper helper = new CommitHelper(repository);
+ asyncExec(() -> {
+ boolean authorEqualsCommitter = commitMessageComponent.getAuthor()
+ .equals(commitMessageComponent.getCommitter());
+ if (authorEqualsCommitter) {
+ commitMessageComponent.setAuthor(helper.getAuthor());
+ }
+ commitMessageComponent.setCommitter(helper.getCommitter());
+ commitMessageComponent.updateUIFromState(false);
+ });
+ }
+
/**
* Resets the commit message component state and saves the overwritten
* commit message into message history
@@ -4285,9 +4315,7 @@ public class StagingView extends ViewPart
InstanceScope.INSTANCE.getNode(
org.eclipse.egit.core.Activator.getPluginId())
.removePreferenceChangeListener(prefListener);
- if (refsChangedListener != null) {
- refsChangedListener.remove();
- }
+ removeRepositoryListeners();
if (switchRepositoriesAction != null) {
switchRepositoriesAction.dispose();