From d0bd6051300f43ba40ab8eff837884ed9fc26b06 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Mon, 24 May 2021 21:46:45 +0200 Subject: [PATCH 1/6] Remove superfluous newline before Co-authored-by trailers --- services/pull/pull.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index cc560fb199d2..92e0a2d44f3e 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -617,13 +617,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) From 6f5eb18d9c36d9d7ce9ef6195a558a5fdf44380a Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Mon, 24 May 2021 21:50:48 +0200 Subject: [PATCH 2/6] 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. --- services/pull/pull.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 92e0a2d44f3e..e1c7e15a6104 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -9,6 +9,7 @@ import ( "bytes" "context" "fmt" + "regexp" "strings" "time" @@ -573,7 +574,10 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string { stringBuilder.WriteString(pr.Issue.Content) if stringBuilder.Len() > 0 { stringBuilder.WriteRune('\n') - stringBuilder.WriteRune('\n') + commitMessageTrailersPattern := regexp.MustCompile(`(^|.*\n\n)([\w-]+: [^\n]+)(\n[\w-]+: [^\n]+)*$`) + if !commitMessageTrailersPattern.MatchString(pr.Issue.Content) { + stringBuilder.WriteRune('\n') + } } // commits list is in reverse chronological order From 48552b355f370fab3d97dbff959a1f4a8e63f2d5 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Wed, 26 May 2021 21:42:16 +0200 Subject: [PATCH 3/6] Reuse compiled regexp --- services/pull/pull.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index e1c7e15a6104..8e2ce4a33b1d 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -517,6 +517,8 @@ func CloseRepoBranchesPulls(doer *models.User, repo *models.Repository) error { return nil } +var commitMessageTrailersPattern = regexp.MustCompile(`(^|.*\n\n)([\w-]+: [^\n]+)(\n[\w-]+: [^\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 { @@ -574,7 +576,6 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string { stringBuilder.WriteString(pr.Issue.Content) if stringBuilder.Len() > 0 { stringBuilder.WriteRune('\n') - commitMessageTrailersPattern := regexp.MustCompile(`(^|.*\n\n)([\w-]+: [^\n]+)(\n[\w-]+: [^\n]+)*$`) if !commitMessageTrailersPattern.MatchString(pr.Issue.Content) { stringBuilder.WriteRune('\n') } From 45b1b785f774d5403b238fac000ba8533c230f71 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Mon, 21 Jun 2021 20:02:08 +0200 Subject: [PATCH 4/6] Simplify regex and deal with trailing \n in PR description --- services/pull/pull.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 704e8bbe94d8..03be2820dcc4 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -517,7 +517,7 @@ func CloseRepoBranchesPulls(doer *models.User, repo *models.Repository) error { return nil } -var commitMessageTrailersPattern = regexp.MustCompile(`(^|.*\n\n)([\w-]+: [^\n]+)(\n[\w-]+: [^\n]+)*$`) +var commitMessageTrailersPattern = regexp.MustCompile(`(?:^|\n\n)(?:[\w-]+: [^\n]+\n*)+$`) // GetSquashMergeCommitMessages returns the commit messages between head and merge base (if there is one) func GetSquashMergeCommitMessages(pr *models.PullRequest) string { @@ -574,12 +574,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') - if !commitMessageTrailersPattern.MatchString(pr.Issue.Content) { - stringBuilder.WriteRune('\n') - } + if !commitMessageTrailersPattern.MatchString(message) { + stringBuilder.WriteRune('\n') + } } } From a84e87e60697eb2cc37e7b108ba770723097d659 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Mon, 21 Jun 2021 22:16:49 +0200 Subject: [PATCH 5/6] Add tests for CommitMessageTrailersPattern - add support for Key:Value (no space after colon) - add support for whitespace "folding" --- services/pull/pull.go | 2 +- services/pull/pull_test.go | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 03be2820dcc4..a39212c341e7 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -517,7 +517,7 @@ func CloseRepoBranchesPulls(doer *models.User, repo *models.Repository) error { return nil } -var commitMessageTrailersPattern = regexp.MustCompile(`(?:^|\n\n)(?:[\w-]+: [^\n]+\n*)+$`) +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 { diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index 64920e355096..c77a8f23636b 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-of-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")) +} From ee89e2a5a6ec67d55839b7d8fa14f9936cfa16cd Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 25 Jun 2021 12:37:19 +0100 Subject: [PATCH 6/6] Update services/pull/pull_test.go Co-authored-by: Norwin --- services/pull/pull_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index c77a8f23636b..81627ebb77bc 100644 --- a/services/pull/pull_test.go +++ b/services/pull/pull_test.go @@ -18,7 +18,7 @@ func TestPullRequest_CommitMessageTrailersPattern(t *testing.T) { 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-of-by: Bob ")) + 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"))