threads / patch / 17895

patchIn the commit dialog, deal with Signed-off-by not on the last line

Subject: [PATCH JGIT] In the commit dialog, deal with Signed-off-by not on the last line

## tl;dr

4 messages between Feb 19, 2009 and Feb 20, 2009. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Yann Simon· Feb 19, 2009, 09:20 UTC · lore

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>
---
I hope this send will not be whitespace damaged.

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

Show changes to diff +40 −26
@@ -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.1.2
Robin Rosenberg· Feb 19, 2009, 17:28 UTC · re: Yann Simon · lore

Re: [PATCH JGIT] In the commit dialog, deal with Signed-off-by not on the last line

Yann Simon <yann.simon.fr@gmail.com>writes:
Show 9 quoted lines
> 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>
> ---
> I hope this send will not be whitespace damaged.
Sorry, It suffers from severe whitespace burns. 
-- robin
Sverre Rabbelier· Feb 19, 2009, 17:53 UTC · re: Yann Simon · lore

Re: [PATCH JGIT] In the commit dialog, deal with Signed-off-by not on the last line

On Thu, Feb 19, 2009 at 10:20, Yann Simon <yann.simon.fr@gmail.com> wrote:
> I hope this send will not be whitespace damaged.

You could of course send your patch inlined, but attach the patch as well if you fear your MUA will damage the patch. That way the patch can still be easily reviewed, and can be applied just as well.

-- 
Cheers,

Sverre Rabbelier
Yann Simon· Feb 20, 2009, 07:48 UTC · re: Sverre Rabbelier · lore

Re: [PATCH JGIT] In the commit dialog, deal with Signed-off-by not on the last line

2009/2/19 Sverre Rabbelier <srabbelier@gmail.com>:
Show 12 quoted lines
> On Thu, Feb 19, 2009 at 10:20, Yann Simon <yann.simon.fr@gmail.com> wrote:
>> I hope this send will not be whitespace damaged.
>
> You could of course send your patch inlined, but attach the patch as
> well if you fear your MUA will damage the patch. That way the patch
> can still be easily reviewed, and can be applied just as well.
>
> --
> Cheers,
>
> Sverre Rabbelier
>

I switched from STMP to IMAP. Maybe it is the problem. I will revert to SMTP and re-test the send.

Sorry for the trouble
Yann

← back to recent threads