Append to existing trailers in generated squash commit message (#15980)

* Remove superfluous newline before Co-authored-by trailers

* Append to existing PR description trailer section

If the existing PR description message already contains a trailer section (e.g. Signed-off-by: ),
append to it instead of creating a new trailer section.

* Reuse compiled regexp

* Simplify regex and deal with trailing \n in PR description

* Add tests for CommitMessageTrailersPattern

- add support for Key:Value (no space after colon)
- add support for whitespace "folding"

* Update services/pull/pull_test.go

Co-authored-by: Norwin <[email protected]>

Co-authored-by: zeripath <[email protected]>
Co-authored-by: Norwin <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>

Author: 5 people

services/pull/pull.go

@@ -9,6 +9,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"regexp"
 	"strings"
 	"time"
 
@@ -528,6 +529,8 @@ func CloseRepoBranchesPulls(doer *models.User, repo *models.Repository) error {
 	return nil
 }
 
+var commitMessageTrailersPattern = regexp.MustCompile(`(?:^|\n\n)(?:[\w-]+[ \t]*:[^\n]+\n*(?:[ \t]+[^\n]+\n*)*)+$`)
+
 // GetSquashMergeCommitMessages returns the commit messages between head and merge base (if there is one)
 func GetSquashMergeCommitMessages(pr *models.PullRequest) string {
 	if err := pr.LoadIssue(); err != nil {
@@ -583,10 +586,13 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string {
 	stringBuilder := strings.Builder{}
 
 	if !setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages {
-		stringBuilder.WriteString(pr.Issue.Content)
+		message := strings.TrimSpace(pr.Issue.Content)
+		stringBuilder.WriteString(message)
 		if stringBuilder.Len() > 0 {
 			stringBuilder.WriteRune('\n')
-			stringBuilder.WriteRune('\n')
+			if !commitMessageTrailersPattern.MatchString(message) {
+				stringBuilder.WriteRune('\n')
+			}
 		}
 	}
 
@@ -657,13 +663,6 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string {
 		}
 	}
 
-	if len(authors) > 0 {
-		if _, err := stringBuilder.WriteRune('\n'); err != nil {
-			log.Error("Unable to write to string builder Error: %v", err)
-			return ""
-		}
-	}
-
 	for _, author := range authors {
 		if _, err := stringBuilder.Write([]byte("Co-authored-by: ")); err != nil {
 			log.Error("Unable to write to string builder Error: %v", err)

services/pull/pull_test.go

@@ -5,4 +5,27 @@
 
 package pull
 
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
 // TODO TestPullRequest_PushToBaseRepo
+
+func TestPullRequest_CommitMessageTrailersPattern(t *testing.T) {
+	// Not a valid trailer section
+	assert.False(t, commitMessageTrailersPattern.MatchString(""))
+	assert.False(t, commitMessageTrailersPattern.MatchString("No trailer."))
+	assert.False(t, commitMessageTrailersPattern.MatchString("Signed-off-by: Bob <[email protected]>\nNot a trailer due to following text."))
+	assert.False(t, commitMessageTrailersPattern.MatchString("Message body not correctly separated from trailer section by empty line.\nSigned-off-by: Bob <[email protected]>"))
+	// Valid trailer section
+	assert.True(t, commitMessageTrailersPattern.MatchString("Signed-off-by: Bob <[email protected]>"))
+	assert.True(t, commitMessageTrailersPattern.MatchString("Signed-off-by: Bob <[email protected]>\nOther-Trailer: Value"))
+	assert.True(t, commitMessageTrailersPattern.MatchString("Message body correctly separated from trailer section by empty line.\n\nSigned-off-by: Bob <[email protected]>"))
+	assert.True(t, commitMessageTrailersPattern.MatchString("Multiple trailers.\n\nSigned-off-by: Bob <[email protected]>\nOther-Trailer: Value"))
+	assert.True(t, commitMessageTrailersPattern.MatchString("Newline after trailer section.\n\nSigned-off-by: Bob <[email protected]>\n"))
+	assert.True(t, commitMessageTrailersPattern.MatchString("No space after colon is accepted.\n\nSigned-off-by:Bob <[email protected]>"))
+	assert.True(t, commitMessageTrailersPattern.MatchString("Additional whitespace is accepted.\n\nSigned-off-by \t :  \tBob   <[email protected]>   "))
+	assert.True(t, commitMessageTrailersPattern.MatchString("Folded value.\n\nFolded-trailer: This is\n a folded\n   trailer value\nOther-Trailer: Value"))
+}