diff --git a/services/pull/pull.go b/services/pull/pull.go index 7b5dd6a964d..db216ddbf45 100644 --- a/services/pull/pull.go +++ b/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) diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index 64920e35509..81627ebb77b 100644 --- a/services/pull/pull_test.go +++ b/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 \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 ")) + // Valid trailer section + assert.True(t, commitMessageTrailersPattern.MatchString("Signed-off-by: Bob ")) + assert.True(t, commitMessageTrailersPattern.MatchString("Signed-off-by: Bob \nOther-Trailer: Value")) + assert.True(t, commitMessageTrailersPattern.MatchString("Message body correctly separated from trailer section by empty line.\n\nSigned-off-by: Bob ")) + assert.True(t, commitMessageTrailersPattern.MatchString("Multiple trailers.\n\nSigned-off-by: Bob \nOther-Trailer: Value")) + assert.True(t, commitMessageTrailersPattern.MatchString("Newline after trailer section.\n\nSigned-off-by: Bob \n")) + assert.True(t, commitMessageTrailersPattern.MatchString("No space after colon is accepted.\n\nSigned-off-by:Bob ")) + assert.True(t, commitMessageTrailersPattern.MatchString("Additional whitespace is accepted.\n\nSigned-off-by \t : \tBob ")) + assert.True(t, commitMessageTrailersPattern.MatchString("Folded value.\n\nFolded-trailer: This is\n a folded\n trailer value\nOther-Trailer: Value")) +}