[PATCH JGIT] Deal with the signed-off in the commit text dialog

[PATCH JGIT] Deal with the signed-off in the commit text dialog

[Yann Simon]

The user can see and edit the signed-off in the commit dialog
before committing.

Updating the committer updates the signed-off.

For new lines in the commit dialog, use Text.DELIMITER for
plateform neutrality.

Signed-off-by: Yann Simon <yann.simon.fr@gmail.com>
---
 .../egit/ui/internal/actions/CommitAction.java     |   10 +--
 .../egit/ui/internal/dialogs/CommitDialog.java     |   88 +++++++++++++++++++-
 .../src/org/spearce/jgit/lib/Constants.java        |    3 +
 3 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
index d619bd7..5996596 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
@@ -171,7 +171,7 @@ private void performCommit(CommitDialog commitDialog, String commitMessage)
 		}
 
 		try {
-			commitMessage = doCommits(commitDialog, commitMessage, treeMap);
+			doCommits(commitDialog, commitMessage, treeMap);
 		} catch (IOException e) {
 			throw new TeamException("Committing changes", e);
 		}
@@ -180,7 +180,7 @@ private void performCommit(CommitDialog commitDialog, String commitMessage)
 		}
 	}
 
-	private String doCommits(CommitDialog commitDialog, String commitMessage,
+	private void doCommits(CommitDialog commitDialog, String commitMessage,
 			HashMap<Repository, Tree> treeMap) throws IOException, TeamException {
 
 		final String author = commitDialog.getAuthor();
@@ -208,11 +208,6 @@ private String doCommits(CommitDialog commitDialog, String commitMessage,
 			}
 			Commit commit = new Commit(repo, parentIds);
 			commit.setTree(tree);
-			commitMessage = commitMessage.replaceAll("\r", "\n");
-			if (commitDialog.isSignedOff())
-				commitMessage += "\n\nSigned-off-by: " + committerIdent.getName() + " <"
-								+ committerIdent.getEmailAddress() + ">";
-
 			commit.setMessage(commitMessage);
 			commit.setAuthor(new PersonIdent(authorIdent, commitDate, timeZone));
 			commit.setCommitter(new PersonIdent(committerIdent, commitDate, timeZone));
@@ -228,7 +223,6 @@ private String doCommits(CommitDialog commitDialog, String commitMessage,
 						+ " to commit " + commit.getCommitId() + ".");
 			}
 		}
-		return commitMessage;
 	}
 
 	private void prepareTrees(IFile[] selectedItems,
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
index 9d062cc..ec1d85d 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
@@ -33,6 +33,8 @@
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.events.KeyAdapter;
 import org.eclipse.swt.events.KeyEvent;
+import org.eclipse.swt.events.ModifyEvent;
+import org.eclipse.swt.events.ModifyListener;
 import org.eclipse.swt.events.SelectionAdapter;
 import org.eclipse.swt.events.SelectionEvent;
 import org.eclipse.swt.events.SelectionListener;
@@ -176,6 +178,18 @@ public void keyPressed(KeyEvent arg0) {
 		committerText.setLayoutData(GridDataFactory.fillDefaults().grab(true, false).create());
 		if (committer != null)
 			committerText.setText(committer);
+		committerText.addModifyListener(new ModifyListener() {
+			public void modifyText(ModifyEvent e) {
+				if (signedOffButton.getSelection()) {
+					// the commit message is signed
+					// the signature must be updated
+					String oldCommitText = commitText.getText();
+					oldCommitText = removeLastLine(oldCommitText);
+					oldCommitText = signOff(oldCommitText);
+					commitText.setText(oldCommitText);
+				}
+			}
+		});
 
 		amendingButton = new Button(container, SWT.CHECK);
 		if (amending) {
@@ -214,6 +228,29 @@ public void widgetDefaultSelected(SelectionEvent arg0) {
 		signedOffButton.setText(UIText.CommitDialog_AddSOB);
 		signedOffButton.setLayoutData(GridDataFactory.fillDefaults().grab(true, false).span(2, 1).create());
 
+		signedOffButton.addSelectionListener(new SelectionListener() {
+			public void widgetSelected(SelectionEvent arg0) {
+				if (signedOffButton.getSelection()) {
+					// add signed off line
+					commitText.setText(signOff(commitText.getText()));
+				} else {
+					// remove signed off line
+					commitText.setText(removeLastLine(commitText.getText()));
+				}
+			}
+
+			public void widgetDefaultSelected(SelectionEvent arg0) {
+				// Empty
+			}
+		});
+
+		commitText.addModifyListener(new ModifyListener() {
+			public void modifyText(ModifyEvent e) {
+				updateSignedOffButton();
+			}
+		});
+		updateSignedOffButton();
+
 		Table resourcesTable = new Table(container, SWT.H_SCROLL | SWT.V_SCROLL
 				| SWT.FULL_SELECTION | SWT.MULTI | SWT.CHECK | SWT.BORDER);
 		resourcesTable.setLayoutData(GridDataFactory.fillDefaults().hint(600,
@@ -241,6 +278,55 @@ public void widgetDefaultSelected(SelectionEvent arg0) {
 		return container;
 	}
 
+	private void updateSignedOffButton() {
+		signedOffButton.setSelection(getLastLine(commitText.getText()).equals(getSignedOff()));
+	}
+
+	private String getSignedOff() {
+		return Constants.SIGNED_OFF_BEGINNING + committerText.getText();
+	}
+
+	private String signOff(String input) {
+		String output = input;
+		if (!output.endsWith(Text.DELIMITER))
+			output += Text.DELIMITER;
+
+		// if the last line is not a signed off (amend a commit), had a line break
+		if (!getLastLine(output).startsWith(Constants.SIGNED_OFF_BEGINNING))
+			output += Text.DELIMITER;
+		output += getSignedOff();
+		return output;
+	}
+
+	private String getLastLine(String input) {
+		String output = removeLastLineBreak(input);
+		int breakLength = Text.DELIMITER.length();
+
+		// get the last line
+		int lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);
+		return lastIndexOfLineBreak == -1 ? output : output.substring(lastIndexOfLineBreak + breakLength, output.length());
+	}
+
+	private String removeLastLine(String input) {
+		String output = removeLastLineBreak(input);
+
+		// remove the last line if possible
+		int lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);
+		return lastIndexOfLineBreak == -1 ? "" : output.substring(0, lastIndexOfLineBreak); //$NON-NLS-1$
+	}
+
+	private String removeLastLineBreak(String input) {
+		String output = input;
+		int breakLength = Text.DELIMITER.length();
+
+		// remove last line break if exist
+		int lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);
+		if (lastIndexOfLineBreak != -1 && lastIndexOfLineBreak == output.length() - breakLength)
+			output = output.substring(0, output.length() - breakLength);
+
+		return output;
+	}
+
 	private Menu getContextMenu() {
 		Menu menu = new Menu(filesViewer.getTable());
 		MenuItem item = new MenuItem(menu, SWT.PUSH);
@@ -330,7 +416,7 @@ private static String getFileStatus(IFile file) {
 	 * @return The message the user entered
 	 */
 	public String getCommitMessage() {
-		return commitMessage;
+		return commitMessage.replaceAll(Text.DELIMITER, "\n"); //$NON-NLS-1$;
 	}
 
 	/**
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
index b0a5b22..13e1012 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
@@ -246,6 +246,9 @@
 	/** Default value for the user name if no other information is available */
 	public static final String UNKNOWN_USER_DEFAULT = "unknown-user";
 
+	/** Beginning of the signed of */
+	public static final String SIGNED_OFF_BEGINNING = "Signed-off-by: ";
+
 	/**
 	 * Create a new digest function for objects.
 	 * 
-- 
1.6.0.4

Re: [PATCH JGIT] Deal with the signed-off in the commit text dialog

[Robin Rosenberg]

tisdag 10 februari 2009 13:55:32 skrev Yann Simon:
> The user can see and edit the signed-off in the commit dialog
> before committing.
> 
> Updating the committer updates the signed-off.
> 
> For new lines in the commit dialog, use Text.DELIMITER for
> plateform neutrality.

Spell "platform"

These is two unrelated changes. I don't deeply about the switch, but
I do want the line end handling so we don't get double spaced messages.

The toggle swithch will happily double the SOB-line. It assumes it is
the last line. Isn't that too strict, though it is the common idiom, but
we also have other types of X-by-lines that we often place below
the SOB line.

Signed-off-by: hacker
Signed-off-by: pschorff
Acked-by: bar

-- robin

[PATCH JGIT] In the commit dialog, deal with Signed-off-by not on the last line (WAS: [PATCH JGIT] Deal with the signed-off in the commit text dialog)

[Yann Simon]

The previous implementation made the assumption that
the Signed-off-by is always on the last line.

Correct this assumption and deal with Signed-off-by everywhere in
the commit message.

Signed-off-by: Yann Simon <yann.simon.fr@gmail.com>
---
Robin Rosenberg wrote:
> tisdag 10 februari 2009 13:55:32 skrev Yann Simon:
> The toggle swithch will happily double the SOB-line. It assumes it is
> the last line. Isn't that too strict, though it is the common idiom, but
> we also have other types of X-by-lines that we often place below
> the SOB line.
>
> Signed-off-by: hacker
> Signed-off-by: pschorff
> Acked-by: bar

This patch should correct this assumption.

-- yann

 .../egit/ui/internal/dialogs/CommitDialog.java     |   65
++++++++++++--------
 1 files changed, 40 insertions(+), 25 deletions(-)

diff --git
a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
index bbe7193..403d69d 100644
---
a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
+++
b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
@@ -179,14 +179,16 @@ public void keyPressed(KeyEvent arg0) {
         if (committer != null)
             committerText.setText(committer);
         committerText.addModifyListener(new ModifyListener() {
+            String oldCommitter = committerText.getText();
             public void modifyText(ModifyEvent e) {
                 if (signedOffButton.getSelection()) {
                     // the commit message is signed
                     // the signature must be updated
-                    String oldCommitText = commitText.getText();
-                    oldCommitText = removeLastLine(oldCommitText);
-                    oldCommitText = signOff(oldCommitText);
-                    commitText.setText(oldCommitText);
+                    String neuCommitter = committerText.getText();
+                    String oldSignOff = getSignedOff(oldCommitter);
+                    String neuSignOff = getSignedOff(neuCommitter);
+                   
commitText.setText(replaceSignOff(commitText.getText(), oldSignOff,
neuSignOff));
+                    oldCommitter = neuCommitter;
                 }
             }
         });
@@ -230,12 +232,16 @@ public void widgetDefaultSelected(SelectionEvent
arg0) {
 
         signedOffButton.addSelectionListener(new SelectionListener() {
             public void widgetSelected(SelectionEvent arg0) {
+                String curText = commitText.getText();
                 if (signedOffButton.getSelection()) {
                     // add signed off line
-                    commitText.setText(signOff(commitText.getText()));
+                    commitText.setText(signOff(curText));
                 } else {
                     // remove signed off line
-                   
commitText.setText(removeLastLine(commitText.getText()));
+                    curText = replaceSignOff(curText, getSignedOff(), "");
+                    if (curText.endsWith(Text.DELIMITER + Text.DELIMITER))
+                        curText = curText.substring(0, curText.length()
- Text.DELIMITER.length());
+                    commitText.setText(curText);
                 }
             }
 
@@ -279,11 +285,19 @@ public void modifyText(ModifyEvent e) {
     }
 
     private void updateSignedOffButton() {
-       
signedOffButton.setSelection(getLastLine(commitText.getText()).equals(getSignedOff()));
+        String curText = commitText.getText();
+        if (!curText.endsWith(Text.DELIMITER))
+            curText += Text.DELIMITER;
+
+        signedOffButton.setSelection(curText.indexOf(getSignedOff() +
Text.DELIMITER) != -1);
     }
 
     private String getSignedOff() {
-        return Constants.SIGNED_OFF_BY_TAG + committerText.getText();
+        return getSignedOff(committerText.getText());
+    }
+
+    private String getSignedOff(String signer) {
+        return Constants.SIGNED_OFF_BY_TAG + signer;
     }
 
     private String signOff(String input) {
@@ -299,32 +313,33 @@ private String signOff(String input) {
     }
 
     private String getLastLine(String input) {
-        String output = removeLastLineBreak(input);
+        String output = input;
         int breakLength = Text.DELIMITER.length();
 
-        // get the last line
+        // remove last line break if exist
         int lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);
+        if (lastIndexOfLineBreak != -1 && lastIndexOfLineBreak ==
output.length() - breakLength)
+            output = output.substring(0, output.length() - breakLength);
+
+        // get the last line
+        lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);
         return lastIndexOfLineBreak == -1 ? output :
output.substring(lastIndexOfLineBreak + breakLength, output.length());
     }
 
-    private String removeLastLine(String input) {
-        String output = removeLastLineBreak(input);
+    private String replaceSignOff(String input, String oldSignOff,
String newSignOff) {
+        assert input != null;
+        assert oldSignOff != null;
+        assert newSignOff != null;
 
-        // remove the last line if possible
-        int lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);
-        return lastIndexOfLineBreak == -1 ? "" : output.substring(0,
lastIndexOfLineBreak); //$NON-NLS-1$
-    }
-
-    private String removeLastLineBreak(String input) {
-        String output = input;
-        int breakLength = Text.DELIMITER.length();
+        String curText = input;
+        if (!curText.endsWith(Text.DELIMITER))
+            curText += Text.DELIMITER;
 
-        // remove last line break if exist
-        int lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);
-        if (lastIndexOfLineBreak != -1 && lastIndexOfLineBreak ==
output.length() - breakLength)
-            output = output.substring(0, output.length() - breakLength);
+        int indexOfSignOff = curText.indexOf(oldSignOff + Text.DELIMITER);
+        if (indexOfSignOff == -1)
+            return input;
 
-        return output;
+        return input.substring(0, indexOfSignOff) + newSignOff +
input.substring(indexOfSignOff + oldSignOff.length(), input.length());
     }
 
     private Menu getContextMenu() {
-- 
1.6.0.4

Re: [PATCH JGIT] In the commit dialog, deal with Signed-off-by not on the last line (WAS: [PATCH JGIT] Deal with the signed-off in the commit text dialog)

[Shawn O. Pearce]

Yann Simon <yann.simon.fr@gmail.com> wrote:
> 
> This patch should correct this assumption.

Maybe, but it can't be applied...
 
> +                    String neuSignOff = getSignedOff(neuCommitter);
> +                   
> commitText.setText(replaceSignOff(commitText.getText(), oldSignOff,
> neuSignOff));
> +                    oldCommitter = neuCommitter;

That's whitespace corruption.


-- 
Shawn.