{"thread":{"id":"17674","subject":"[PATCH JGIT] Add the signed-off in the commit text dialog","startedAt":"2009-02-09T15:17:17Z","lastAt":"2009-02-09T15:58:18Z","messageCount":4,"participants":["Yann Simon","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103847","messageId":"499048FD.7050803@gmail.com","threadId":"17674","inReplyTo":null,"subject":"[PATCH JGIT] Add the signed-off in the commit text dialog","fromName":"Yann Simon","fromEmail":"yann.simon.fr@gmail.com","sentAt":"2009-02-09T15:17:17Z","receivedAt":"2009-02-09T15:17:17Z","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\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---\nThis patch only applies after the 2 previous patches.\nIf you want to, I could probably modify this patch so that it would\napply on the current origin.\n\n .../egit/ui/internal/actions/CommitAction.java     |   10 +-----\n .../egit/ui/internal/dialogs/CommitDialog.java     |   29 +++++++++++++++++++-\n 2 files changed, 30 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 97aa60f..6aff07e 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@@ -172,7 +172,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@@ -181,7 +181,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\tString author = commitDialog.getAuthor();\n@@ -209,11 +209,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@@ -229,7 +224,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..8f85c08 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@@ -17,6 +17,7 @@\n import java.util.Collections;\n import java.util.Comparator;\n import java.util.Iterator;\n+import java.util.regex.Pattern;\n \n import org.eclipse.core.resources.IFile;\n import org.eclipse.core.resources.IProject;\n@@ -67,6 +68,8 @@\n  */\n public class CommitDialog extends Dialog {\n \n+\tprivate static Pattern signedOffPattern = Pattern.compile(\"(.|\\r|\\n)*Signed-off-by: .*(\\r|\\n)*\"); //$NON-NLS-1$\n+\n \tclass CommitContentProvider implements IStructuredContentProvider {\n \n \t\tpublic void inputChanged(Viewer viewer, Object oldInput, Object newInput) {\n@@ -214,6 +217,30 @@ 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\tboolean alreadySigned = false;\n+\t\t\tpublic void widgetSelected(SelectionEvent arg0) {\n+\t\t\t\tif (alreadySigned)\n+\t\t\t\t\treturn;\n+\t\t\t\tif (signedOffButton.getSelection()) {\n+\t\t\t\t\talreadySigned = true;\n+\t\t\t\t\tString curText = commitText.getText();\n+\n+\t\t\t\t\t// add new lines if necessary\n+\t\t\t\t\tif (!curText.endsWith(Text.DELIMITER))\n+\t\t\t\t\t\tcurText += Text.DELIMITER;\n+\n+\t\t\t\t\t// if the last line is not a signed off (amend a commit), add a line break\n+\t\t\t\t\tif (!signedOffPattern.matcher(new StringBuilder(curText)).matches())\n+\t\t\t\t\t\tcurText += Text.DELIMITER;\n+\t\t\t\t\tcommitText.setText(curText + \"Signed-off-by: \" + committerText.getText()); //$NON-NLS-1$\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 \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@@ -330,7 +357,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/**\n-- \n1.6.0.4\n"},{"id":"103853","messageId":"20090209154627.GJ30949@spearce.org","threadId":"17674","inReplyTo":"499048FD.7050803@gmail.com","subject":"Re: [PATCH JGIT] Add the signed-off in the commit text dialog","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-09T15:46:27Z","receivedAt":"2009-02-09T15:46:27Z","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> The user can see and edit the signed-off in the commit dialog\n> before committing.\n> \n> For new lines in the commit dialog, use Text.DELIMITER for\n> plateform neutrality.\n> \n> Signed-off-by: Yann Simon <yann.simon.fr@gmail.com>\n> ---\n> This patch only applies after the 2 previous patches.\n> If you want to, I could probably modify this patch so that it would\n> apply on the current origin.\n\nThe other two have been applied so no need to rebase.\n \n> 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\n> index 9d062cc..8f85c08 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> @@ -67,6 +68,8 @@\n>   */\n>  public class CommitDialog extends Dialog {\n>  \n> +\tprivate static Pattern signedOffPattern = Pattern.compile(\"(.|\\r|\\n)*Signed-off-by: .*(\\r|\\n)*\"); //$NON-NLS-1$\n\nWouldn't \"[.\\r\\n]\" be easier to use here than \"(.|\\r|\\n)\"?\n\n> @@ -214,6 +217,30 @@ 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\tboolean alreadySigned = false;\n> +\t\t\tpublic void widgetSelected(SelectionEvent arg0) {\n> +\t\t\t\tif (alreadySigned)\n> +\t\t\t\t\treturn;\n> +\t\t\t\tif (signedOffButton.getSelection()) {\n> +\t\t\t\t\talreadySigned = true;\n\nHuh.  So I can only push the checkbox once, and that after that\nits just an idiot switch?\n\nIf that's really going to be how it is, maybe we should disable\nthe checkbox?\n\nFWIW, git-gui actually looks for the user's Signed-off-by line in the\ntext buffer.  If it can't find it, then it appends it onto the end.\nThat way the user can delete the line and do the sign off again if\nthey messed up somehow.\n\nAnd actually, given that this is a checkbox and not a button, maybe\nwe should be able to *delete* the SBO line when the user tries to\nuncheck the checkbox.  Which then gets into, what if the user made\nan edit to the text and changed the SBO line, should this box get\nunchecked automatically by some form a listener on the text box?\n\nFood for thought.  I'm not sure what it should be.  But if it were\na checkbox, as a user I'd like it to be bi-directional (both add\nand remove my SBO) and also uncheck when I edit or delete the SBO\nline in the message box.\n\n-- \nShawn.\n"},{"id":"103855","messageId":"551f769b0902090750t2d6a43c4vb4944df340fc5148@mail.gmail.com","threadId":"17674","inReplyTo":"20090209154627.GJ30949@spearce.org","subject":"Re: [PATCH JGIT] Add the signed-off in the commit text dialog","fromName":"Yann Simon","fromEmail":"yann.simon.fr@gmail.com","sentAt":"2009-02-09T15:50:18Z","receivedAt":"2009-02-09T15:50:18Z","isPatch":true,"sender":{"key":"yann.simon.fr@gmail.com","avatar":"https://gravatar.com/avatar/2d926895d27ac988c5c8e591887e5a6a4c7036390403c74fce92519119b887a0?d=mp&s=160"},"body":"2009/2/9 Shawn O. Pearce <spearce@spearce.org>:\n> Yann Simon <yann.simon.fr@gmail.com> wrote:\n>> The user can see and edit the signed-off in the commit dialog\n>> before committing.\n>>\n>> For new lines in the commit dialog, use Text.DELIMITER for\n>> plateform neutrality.\n>>\n>> Signed-off-by: Yann Simon <yann.simon.fr@gmail.com>\n>> ---\n>> This patch only applies after the 2 previous patches.\n>> If you want to, I could probably modify this patch so that it would\n>> apply on the current origin.\n>\n> The other two have been applied so no need to rebase.\n>\n>> 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\n>> index 9d062cc..8f85c08 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>> @@ -67,6 +68,8 @@\n>>   */\n>>  public class CommitDialog extends Dialog {\n>>\n>> +     private static Pattern signedOffPattern = Pattern.compile(\"(.|\\r|\\n)*Signed-off-by: .*(\\r|\\n)*\"); //$NON-NLS-1$\n>\n> Wouldn't \"[.\\r\\n]\" be easier to use here than \"(.|\\r|\\n)\"?\n>\n>> @@ -214,6 +217,30 @@ public void widgetDefaultSelected(SelectionEvent arg0) {\n>>               signedOffButton.setText(UIText.CommitDialog_AddSOB);\n>>               signedOffButton.setLayoutData(GridDataFactory.fillDefaults().grab(true, false).span(2, 1).create());\n>>\n>> +             signedOffButton.addSelectionListener(new SelectionListener() {\n>> +                     boolean alreadySigned = false;\n>> +                     public void widgetSelected(SelectionEvent arg0) {\n>> +                             if (alreadySigned)\n>> +                                     return;\n>> +                             if (signedOffButton.getSelection()) {\n>> +                                     alreadySigned = true;\n>\n> Huh.  So I can only push the checkbox once, and that after that\n> its just an idiot switch?\n>\n> If that's really going to be how it is, maybe we should disable\n> the checkbox?\n>\n> FWIW, git-gui actually looks for the user's Signed-off-by line in the\n> text buffer.  If it can't find it, then it appends it onto the end.\n> That way the user can delete the line and do the sign off again if\n> they messed up somehow.\n>\n> And actually, given that this is a checkbox and not a button, maybe\n> we should be able to *delete* the SBO line when the user tries to\n> uncheck the checkbox.  Which then gets into, what if the user made\n> an edit to the text and changed the SBO line, should this box get\n> unchecked automatically by some form a listener on the text box?\n>\n> Food for thought.  I'm not sure what it should be.  But if it were\n> a checkbox, as a user I'd like it to be bi-directional (both add\n> and remove my SBO) and also uncheck when I edit or delete the SBO\n> line in the message box.\n\nHe he, more challenging but it would be much better too!\nI try to implement this when I have time.\n\nAnd what should we do, when we commit a change from somebody else?\nSould we be able to modify the signed-off of the author?\n\nYann\n"},{"id":"103857","messageId":"20090209155818.GK30949@spearce.org","threadId":"17674","inReplyTo":"551f769b0902090750t2d6a43c4vb4944df340fc5148@mail.gmail.com","subject":"Re: [PATCH JGIT] Add the signed-off in the commit text dialog","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-09T15:58:18Z","receivedAt":"2009-02-09T15:58:18Z","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> And what should we do, when we commit a change from somebody else?\n> Sould we be able to modify the signed-off of the author?\n\nWell, not by a fancy UI widget and automated tools.  But editing\nthe SBO line in the message buffer is fine, just like any other\npart of the message.\n\nIMHO, the SBO checkbox/button in the UI is only for *your* SBO,\nas the committer.  That's how git-gui behaves and it seems to\nwork out nicely.\n\n-- \nShawn.\n"}]}