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

> Yann Simon <yann.simon.fr@gmail.com> wrote:
>> The user can see and edit the signed-off in the commit dialog
>> before committing.
>>
>> For new lines in the commit dialog, use Text.DELIMITER for
>> plateform neutrality.
>>
>> Signed-off-by: Yann Simon <yann.simon.fr@gmail.com>
>> ---
>> This patch only applies after the 2 previous patches.
>> If you want to, I could probably modify this patch so that it would
>> apply on the current origin.
>
> The other two have been applied so no need to rebase.
>
>> 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..8f85c08 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
>> @@ -67,6 +68,8 @@
>>   */
>>  public class CommitDialog extends Dialog {
>>
>> +     private static Pattern signedOffPattern = Pattern.compile("(.|\r|\n)*Signed-off-by: .*(\r|\n)*"); //$NON-NLS-1$
>
> Wouldn't "[.\r\n]" be easier to use here than "(.|\r|\n)"?
>
>> @@ -214,6 +217,30 @@ 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() {
>> +                     boolean alreadySigned = false;
>> +                     public void widgetSelected(SelectionEvent arg0) {
>> +                             if (alreadySigned)
>> +                                     return;
>> +                             if (signedOffButton.getSelection()) {
>> +                                     alreadySigned = true;
>
> Huh.  So I can only push the checkbox once, and that after that
> its just an idiot switch?
>
> If that's really going to be how it is, maybe we should disable
> the checkbox?
>
> FWIW, git-gui actually looks for the user's Signed-off-by line in the
> text buffer.  If it can't find it, then it appends it onto the end.
> That way the user can delete the line and do the sign off again if
> they messed up somehow.
>
> And actually, given that this is a checkbox and not a button, maybe
> we should be able to *delete* the SBO line when the user tries to
> uncheck the checkbox.  Which then gets into, what if the user made
> an edit to the text and changed the SBO line, should this box get
> unchecked automatically by some form a listener on the text box?
>
> Food for thought.  I'm not sure what it should be.  But if it were
> a checkbox, as a user I'd like it to be bi-directional (both add
> and remove my SBO) and also uncheck when I edit or delete the SBO
> line in the message box.