{"thread":{"id":"17696","subject":"[PATCH JGIT] Deal with the signed-off in the commit text dialog","startedAt":"2009-02-10T12:55:32Z","lastAt":"2009-02-11T19:40:58Z","messageCount":4,"participants":["Yann Simon","Robin Rosenberg","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103994","messageId":"49917944.6030309@gmail.com","threadId":"17696","inReplyTo":null,"subject":"[PATCH JGIT] Deal with the signed-off in the commit text dialog","fromName":"Yann Simon","fromEmail":"yann.simon.fr@gmail.com","sentAt":"2009-02-10T12:55:32Z","receivedAt":"2009-02-10T12:55:32Z","isPatch":true,"sender":{"key":"yann.simon.fr@gmail.com","avatar":"https://gravatar.com/avatar/2d926895d27ac988c5c8e591887e5a6a4c7036390403c74fce92519119b887a0?d=mp&s=160"},"body":"The user can see and edit the signed-off in the commit dialog\nbefore committing.\n\nUpdating the committer updates the signed-off.\n\nFor new lines in the commit dialog, use Text.DELIMITER for\nplateform neutrality.\n\nSigned-off-by: Yann Simon <yann.simon.fr@gmail.com>\n---\n .../egit/ui/internal/actions/CommitAction.java     |   10 +--\n .../egit/ui/internal/dialogs/CommitDialog.java     |   88 +++++++++++++++++++-\n .../src/org/spearce/jgit/lib/Constants.java        |    3 +\n 3 files changed, 92 insertions(+), 9 deletions(-)\n\ndiff --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\nindex d619bd7..5996596 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java\n@@ -171,7 +171,7 @@ private void performCommit(CommitDialog commitDialog, String commitMessage)\n \t\t}\n \n \t\ttry {\n-\t\t\tcommitMessage = doCommits(commitDialog, commitMessage, treeMap);\n+\t\t\tdoCommits(commitDialog, commitMessage, treeMap);\n \t\t} catch (IOException e) {\n \t\t\tthrow new TeamException(\"Committing changes\", e);\n \t\t}\n@@ -180,7 +180,7 @@ private void performCommit(CommitDialog commitDialog, String commitMessage)\n \t\t}\n \t}\n \n-\tprivate String doCommits(CommitDialog commitDialog, String commitMessage,\n+\tprivate void doCommits(CommitDialog commitDialog, String commitMessage,\n \t\t\tHashMap<Repository, Tree> treeMap) throws IOException, TeamException {\n \n \t\tfinal String author = commitDialog.getAuthor();\n@@ -208,11 +208,6 @@ private String doCommits(CommitDialog commitDialog, String commitMessage,\n \t\t\t}\n \t\t\tCommit commit = new Commit(repo, parentIds);\n \t\t\tcommit.setTree(tree);\n-\t\t\tcommitMessage = commitMessage.replaceAll(\"\\r\", \"\\n\");\n-\t\t\tif (commitDialog.isSignedOff())\n-\t\t\t\tcommitMessage += \"\\n\\nSigned-off-by: \" + committerIdent.getName() + \" <\"\n-\t\t\t\t\t\t\t\t+ committerIdent.getEmailAddress() + \">\";\n-\n \t\t\tcommit.setMessage(commitMessage);\n \t\t\tcommit.setAuthor(new PersonIdent(authorIdent, commitDate, timeZone));\n \t\t\tcommit.setCommitter(new PersonIdent(committerIdent, commitDate, timeZone));\n@@ -228,7 +223,6 @@ private String doCommits(CommitDialog commitDialog, String commitMessage,\n \t\t\t\t\t\t+ \" to commit \" + commit.getCommitId() + \".\");\n \t\t\t}\n \t\t}\n-\t\treturn commitMessage;\n \t}\n \n \tprivate void prepareTrees(IFile[] selectedItems,\ndiff --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\nindex 9d062cc..ec1d85d 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java\n@@ -33,6 +33,8 @@\n import org.eclipse.swt.SWT;\n import org.eclipse.swt.events.KeyAdapter;\n import org.eclipse.swt.events.KeyEvent;\n+import org.eclipse.swt.events.ModifyEvent;\n+import org.eclipse.swt.events.ModifyListener;\n import org.eclipse.swt.events.SelectionAdapter;\n import org.eclipse.swt.events.SelectionEvent;\n import org.eclipse.swt.events.SelectionListener;\n@@ -176,6 +178,18 @@ public void keyPressed(KeyEvent arg0) {\n \t\tcommitterText.setLayoutData(GridDataFactory.fillDefaults().grab(true, false).create());\n \t\tif (committer != null)\n \t\t\tcommitterText.setText(committer);\n+\t\tcommitterText.addModifyListener(new ModifyListener() {\n+\t\t\tpublic void modifyText(ModifyEvent e) {\n+\t\t\t\tif (signedOffButton.getSelection()) {\n+\t\t\t\t\t// the commit message is signed\n+\t\t\t\t\t// the signature must be updated\n+\t\t\t\t\tString oldCommitText = commitText.getText();\n+\t\t\t\t\toldCommitText = removeLastLine(oldCommitText);\n+\t\t\t\t\toldCommitText = signOff(oldCommitText);\n+\t\t\t\t\tcommitText.setText(oldCommitText);\n+\t\t\t\t}\n+\t\t\t}\n+\t\t});\n \n \t\tamendingButton = new Button(container, SWT.CHECK);\n \t\tif (amending) {\n@@ -214,6 +228,29 @@ public void widgetDefaultSelected(SelectionEvent arg0) {\n \t\tsignedOffButton.setText(UIText.CommitDialog_AddSOB);\n \t\tsignedOffButton.setLayoutData(GridDataFactory.fillDefaults().grab(true, false).span(2, 1).create());\n \n+\t\tsignedOffButton.addSelectionListener(new SelectionListener() {\n+\t\t\tpublic void widgetSelected(SelectionEvent arg0) {\n+\t\t\t\tif (signedOffButton.getSelection()) {\n+\t\t\t\t\t// add signed off line\n+\t\t\t\t\tcommitText.setText(signOff(commitText.getText()));\n+\t\t\t\t} else {\n+\t\t\t\t\t// remove signed off line\n+\t\t\t\t\tcommitText.setText(removeLastLine(commitText.getText()));\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\tpublic void widgetDefaultSelected(SelectionEvent arg0) {\n+\t\t\t\t// Empty\n+\t\t\t}\n+\t\t});\n+\n+\t\tcommitText.addModifyListener(new ModifyListener() {\n+\t\t\tpublic void modifyText(ModifyEvent e) {\n+\t\t\t\tupdateSignedOffButton();\n+\t\t\t}\n+\t\t});\n+\t\tupdateSignedOffButton();\n+\n \t\tTable resourcesTable = new Table(container, SWT.H_SCROLL | SWT.V_SCROLL\n \t\t\t\t| SWT.FULL_SELECTION | SWT.MULTI | SWT.CHECK | SWT.BORDER);\n \t\tresourcesTable.setLayoutData(GridDataFactory.fillDefaults().hint(600,\n@@ -241,6 +278,55 @@ public void widgetDefaultSelected(SelectionEvent arg0) {\n \t\treturn container;\n \t}\n \n+\tprivate void updateSignedOffButton() {\n+\t\tsignedOffButton.setSelection(getLastLine(commitText.getText()).equals(getSignedOff()));\n+\t}\n+\n+\tprivate String getSignedOff() {\n+\t\treturn Constants.SIGNED_OFF_BEGINNING + committerText.getText();\n+\t}\n+\n+\tprivate String signOff(String input) {\n+\t\tString output = input;\n+\t\tif (!output.endsWith(Text.DELIMITER))\n+\t\t\toutput += Text.DELIMITER;\n+\n+\t\t// if the last line is not a signed off (amend a commit), had a line break\n+\t\tif (!getLastLine(output).startsWith(Constants.SIGNED_OFF_BEGINNING))\n+\t\t\toutput += Text.DELIMITER;\n+\t\toutput += getSignedOff();\n+\t\treturn output;\n+\t}\n+\n+\tprivate String getLastLine(String input) {\n+\t\tString output = removeLastLineBreak(input);\n+\t\tint breakLength = Text.DELIMITER.length();\n+\n+\t\t// get the last line\n+\t\tint lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);\n+\t\treturn lastIndexOfLineBreak == -1 ? output : output.substring(lastIndexOfLineBreak + breakLength, output.length());\n+\t}\n+\n+\tprivate String removeLastLine(String input) {\n+\t\tString output = removeLastLineBreak(input);\n+\n+\t\t// remove the last line if possible\n+\t\tint lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);\n+\t\treturn lastIndexOfLineBreak == -1 ? \"\" : output.substring(0, lastIndexOfLineBreak); //$NON-NLS-1$\n+\t}\n+\n+\tprivate String removeLastLineBreak(String input) {\n+\t\tString output = input;\n+\t\tint breakLength = Text.DELIMITER.length();\n+\n+\t\t// remove last line break if exist\n+\t\tint lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);\n+\t\tif (lastIndexOfLineBreak != -1 && lastIndexOfLineBreak == output.length() - breakLength)\n+\t\t\toutput = output.substring(0, output.length() - breakLength);\n+\n+\t\treturn output;\n+\t}\n+\n \tprivate Menu getContextMenu() {\n \t\tMenu menu = new Menu(filesViewer.getTable());\n \t\tMenuItem item = new MenuItem(menu, SWT.PUSH);\n@@ -330,7 +416,7 @@ private static String getFileStatus(IFile file) {\n \t * @return The message the user entered\n \t */\n \tpublic String getCommitMessage() {\n-\t\treturn commitMessage;\n+\t\treturn commitMessage.replaceAll(Text.DELIMITER, \"\\n\"); //$NON-NLS-1$;\n \t}\n \n \t/**\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java\nindex b0a5b22..13e1012 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java\n@@ -246,6 +246,9 @@\n \t/** Default value for the user name if no other information is available */\n \tpublic static final String UNKNOWN_USER_DEFAULT = \"unknown-user\";\n \n+\t/** Beginning of the signed of */\n+\tpublic static final String SIGNED_OFF_BEGINNING = \"Signed-off-by: \";\n+\n \t/**\n \t * Create a new digest function for objects.\n \t * \n-- \n1.6.0.4\n"},{"id":"104128","messageId":"200902110116.36172.robin.rosenberg.lists@dewire.com","threadId":"17696","inReplyTo":"49917944.6030309@gmail.com","subject":"Re: [PATCH JGIT] Deal with the signed-off in the commit text dialog","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-02-11T00:16:35Z","receivedAt":"2009-02-11T00:16:35Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"tisdag 10 februari 2009 13:55:32 skrev Yann Simon:\n> The user can see and edit the signed-off in the commit dialog\n> before committing.\n> \n> Updating the committer updates the signed-off.\n> \n> For new lines in the commit dialog, use Text.DELIMITER for\n> plateform neutrality.\n\nSpell \"platform\"\n\nThese is two unrelated changes. I don't deeply about the switch, but\nI do want the line end handling so we don't get double spaced messages.\n\nThe toggle swithch will happily double the SOB-line. It assumes it is\nthe last line. Isn't that too strict, though it is the common idiom, but\nwe also have other types of X-by-lines that we often place below\nthe SOB line.\n\nSigned-off-by: hacker\nSigned-off-by: pschorff\nAcked-by: bar\n\n-- robin\n"},{"id":"104214","messageId":"499304A2.1060908@gmail.com","threadId":"17696","inReplyTo":"200902110116.36172.robin.rosenberg.lists@dewire.com","subject":"[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)","fromName":"Yann Simon","fromEmail":"yann.simon.fr@gmail.com","sentAt":"2009-02-11T17:02:26Z","receivedAt":"2009-02-11T17:02:26Z","isPatch":true,"sender":{"key":"yann.simon.fr@gmail.com","avatar":"https://gravatar.com/avatar/2d926895d27ac988c5c8e591887e5a6a4c7036390403c74fce92519119b887a0?d=mp&s=160"},"body":"The previous implementation made the assumption that\nthe Signed-off-by is always on the last line.\n\nCorrect this assumption and deal with Signed-off-by everywhere in\nthe commit message.\n\nSigned-off-by: Yann Simon <yann.simon.fr@gmail.com>\n---\nRobin Rosenberg wrote:\n> tisdag 10 februari 2009 13:55:32 skrev Yann Simon:\n> The toggle swithch will happily double the SOB-line. It assumes it is\n> the last line. Isn't that too strict, though it is the common idiom, but\n> we also have other types of X-by-lines that we often place below\n> the SOB line.\n>\n> Signed-off-by: hacker\n> Signed-off-by: pschorff\n> Acked-by: bar\n\nThis patch should correct this assumption.\n\n-- yann\n\n .../egit/ui/internal/dialogs/CommitDialog.java     |   65\n++++++++++++--------\n 1 files changed, 40 insertions(+), 25 deletions(-)\n\ndiff --git\na/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java\nb/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java\nindex bbe7193..403d69d 100644\n---\na/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java\n+++\nb/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java\n@@ -179,14 +179,16 @@ public void keyPressed(KeyEvent arg0) {\n         if (committer != null)\n             committerText.setText(committer);\n         committerText.addModifyListener(new ModifyListener() {\n+            String oldCommitter = committerText.getText();\n             public void modifyText(ModifyEvent e) {\n                 if (signedOffButton.getSelection()) {\n                     // the commit message is signed\n                     // the signature must be updated\n-                    String oldCommitText = commitText.getText();\n-                    oldCommitText = removeLastLine(oldCommitText);\n-                    oldCommitText = signOff(oldCommitText);\n-                    commitText.setText(oldCommitText);\n+                    String neuCommitter = committerText.getText();\n+                    String oldSignOff = getSignedOff(oldCommitter);\n+                    String neuSignOff = getSignedOff(neuCommitter);\n+                   \ncommitText.setText(replaceSignOff(commitText.getText(), oldSignOff,\nneuSignOff));\n+                    oldCommitter = neuCommitter;\n                 }\n             }\n         });\n@@ -230,12 +232,16 @@ public void widgetDefaultSelected(SelectionEvent\narg0) {\n \n         signedOffButton.addSelectionListener(new SelectionListener() {\n             public void widgetSelected(SelectionEvent arg0) {\n+                String curText = commitText.getText();\n                 if (signedOffButton.getSelection()) {\n                     // add signed off line\n-                    commitText.setText(signOff(commitText.getText()));\n+                    commitText.setText(signOff(curText));\n                 } else {\n                     // remove signed off line\n-                   \ncommitText.setText(removeLastLine(commitText.getText()));\n+                    curText = replaceSignOff(curText, getSignedOff(), \"\");\n+                    if (curText.endsWith(Text.DELIMITER + Text.DELIMITER))\n+                        curText = curText.substring(0, curText.length()\n- Text.DELIMITER.length());\n+                    commitText.setText(curText);\n                 }\n             }\n \n@@ -279,11 +285,19 @@ public void modifyText(ModifyEvent e) {\n     }\n \n     private void updateSignedOffButton() {\n-       \nsignedOffButton.setSelection(getLastLine(commitText.getText()).equals(getSignedOff()));\n+        String curText = commitText.getText();\n+        if (!curText.endsWith(Text.DELIMITER))\n+            curText += Text.DELIMITER;\n+\n+        signedOffButton.setSelection(curText.indexOf(getSignedOff() +\nText.DELIMITER) != -1);\n     }\n \n     private String getSignedOff() {\n-        return Constants.SIGNED_OFF_BY_TAG + committerText.getText();\n+        return getSignedOff(committerText.getText());\n+    }\n+\n+    private String getSignedOff(String signer) {\n+        return Constants.SIGNED_OFF_BY_TAG + signer;\n     }\n \n     private String signOff(String input) {\n@@ -299,32 +313,33 @@ private String signOff(String input) {\n     }\n \n     private String getLastLine(String input) {\n-        String output = removeLastLineBreak(input);\n+        String output = input;\n         int breakLength = Text.DELIMITER.length();\n \n-        // get the last line\n+        // remove last line break if exist\n         int lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);\n+        if (lastIndexOfLineBreak != -1 && lastIndexOfLineBreak ==\noutput.length() - breakLength)\n+            output = output.substring(0, output.length() - breakLength);\n+\n+        // get the last line\n+        lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);\n         return lastIndexOfLineBreak == -1 ? output :\noutput.substring(lastIndexOfLineBreak + breakLength, output.length());\n     }\n \n-    private String removeLastLine(String input) {\n-        String output = removeLastLineBreak(input);\n+    private String replaceSignOff(String input, String oldSignOff,\nString newSignOff) {\n+        assert input != null;\n+        assert oldSignOff != null;\n+        assert newSignOff != null;\n \n-        // remove the last line if possible\n-        int lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);\n-        return lastIndexOfLineBreak == -1 ? \"\" : output.substring(0,\nlastIndexOfLineBreak); //$NON-NLS-1$\n-    }\n-\n-    private String removeLastLineBreak(String input) {\n-        String output = input;\n-        int breakLength = Text.DELIMITER.length();\n+        String curText = input;\n+        if (!curText.endsWith(Text.DELIMITER))\n+            curText += Text.DELIMITER;\n \n-        // remove last line break if exist\n-        int lastIndexOfLineBreak = output.lastIndexOf(Text.DELIMITER);\n-        if (lastIndexOfLineBreak != -1 && lastIndexOfLineBreak ==\noutput.length() - breakLength)\n-            output = output.substring(0, output.length() - breakLength);\n+        int indexOfSignOff = curText.indexOf(oldSignOff + Text.DELIMITER);\n+        if (indexOfSignOff == -1)\n+            return input;\n \n-        return output;\n+        return input.substring(0, indexOfSignOff) + newSignOff +\ninput.substring(indexOfSignOff + oldSignOff.length(), input.length());\n     }\n \n     private Menu getContextMenu() {\n-- \n1.6.0.4\n"},{"id":"104283","messageId":"20090211194058.GV30949@spearce.org","threadId":"17696","inReplyTo":"499304A2.1060908@gmail.com","subject":"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)","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-11T19:40:58Z","receivedAt":"2009-02-11T19:40:58Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Yann Simon <yann.simon.fr@gmail.com> wrote:\n> \n> This patch should correct this assumption.\n\nMaybe, but it can't be applied...\n \n> +                    String neuSignOff = getSignedOff(neuCommitter);\n> +                   \n> commitText.setText(replaceSignOff(commitText.getText(), oldSignOff,\n> neuSignOff));\n> +                    oldCommitter = neuCommitter;\n\nThat's whitespace corruption.\n\n\n-- \nShawn.\n"}]}