From cfd13c1bfa4f655663b1f0f4ee07ee70f345d02a Mon Sep 17 00:00:00 2001 From: Jim Lin Date: Wed, 21 May 2025 09:21:22 +0800 Subject: [PATCH 1/5] Add "\n" after title unconditionally in case the message is empty If the message from form.MergeMessageField is empty, we will miss a "\n" between the title and the "Co-authored-by:" line. The title and message should have a blank line between of them. --- routers/web/repo/pull.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 43ddc265cfafb..012c75983e17b 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1109,8 +1109,9 @@ func MergePullRequest(ctx *context.Context) { } form.MergeMessageField = strings.TrimSpace(form.MergeMessageField) + message += "\n" if len(form.MergeMessageField) > 0 { - message += "\n\n" + form.MergeMessageField + message += "\n" + form.MergeMessageField } if form.MergeWhenChecksSucceed { From 669cccd0a8070e5b9ca0185cfecff23233371660 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 26 May 2025 18:53:22 +0800 Subject: [PATCH 2/5] fix --- routers/web/repo/pull.go | 3 +-- services/pull/merge_squash.go | 3 +++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 012c75983e17b..43ddc265cfafb 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1109,9 +1109,8 @@ func MergePullRequest(ctx *context.Context) { } form.MergeMessageField = strings.TrimSpace(form.MergeMessageField) - message += "\n" if len(form.MergeMessageField) > 0 { - message += "\n" + form.MergeMessageField + message += "\n\n" + form.MergeMessageField } if form.MergeWhenChecksSucceed { diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index 72660cd3c57d4..8f4bb7e201857 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -66,6 +66,9 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error { if setting.Repository.PullRequest.AddCoCommitterTrailers && ctx.committer.String() != sig.String() { // add trailer + if !strings.HasSuffix(message, "\n") { + message += "\n" + } if !strings.Contains(message, "Co-authored-by: "+sig.String()) { message += "\nCo-authored-by: " + sig.String() } From 26e1f785e26f561be7b8e4e1766e2d9337596ba9 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 27 May 2025 15:18:08 +0800 Subject: [PATCH 3/5] fix --- services/pull/merge_squash.go | 38 ++++++++++++++++++++++++++++------- services/pull/merge_test.go | 12 +++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index 8f4bb7e201857..7042088b079c1 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -6,6 +6,7 @@ package pull import ( "fmt" "strings" + "unicode" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -51,6 +52,34 @@ func getAuthorSignatureSquash(ctx *mergeContext) (*git.Signature, error) { return ctx.pr.Issue.Poster.NewGitSig(), nil } +func AddCommitMessageTailer(message, tailerKey, tailerValue string) string { + tailerLine := tailerKey + ": " + tailerValue + if strings.Contains(message, "\n"+tailerLine+"\n") || strings.HasSuffix(message, "\n"+tailerLine) { + return message + } + + if !strings.HasSuffix(message, "\n") { + message += "\n" + } + pos1 := strings.LastIndexByte(message[:len(message)-1], '\n') + pos2 := -1 + if pos1 != -1 { + pos2 = strings.IndexByte(message[pos1:], ':') + if pos2 != -1 { + pos2 += pos1 + } + } + var lastLine string + if pos1 != -1 && pos2 != -1 { + lastLine = message[pos1+1 : pos2+1] + } + lastLineIsTailerLine := lastLine != "" && unicode.IsUpper(rune(lastLine[0])) && strings.Contains(lastLine, "-") + if !strings.HasSuffix(message, "\n\n") && !lastLineIsTailerLine { + message += "\n" + } + return message + tailerLine +} + // doMergeStyleSquash squashes the tracking branch on the current HEAD (=base) func doMergeStyleSquash(ctx *mergeContext, message string) error { sig, err := getAuthorSignatureSquash(ctx) @@ -66,13 +95,8 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error { if setting.Repository.PullRequest.AddCoCommitterTrailers && ctx.committer.String() != sig.String() { // add trailer - if !strings.HasSuffix(message, "\n") { - message += "\n" - } - if !strings.Contains(message, "Co-authored-by: "+sig.String()) { - message += "\nCo-authored-by: " + sig.String() - } - message += fmt.Sprintf("\nCo-committed-by: %s\n", sig.String()) + message = AddCommitMessageTailer(message, "Co-authored-by", sig.String()) + message = AddCommitMessageTailer(message, "Co-committed-by", sig.String()) // FIXME: this one should be removed, it is not really used or widely used } cmdCommit := git.NewCommand("commit"). AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). diff --git a/services/pull/merge_test.go b/services/pull/merge_test.go index 6df6f55d46115..3630ee92a5fdd 100644 --- a/services/pull/merge_test.go +++ b/services/pull/merge_test.go @@ -65,3 +65,15 @@ func Test_expandDefaultMergeMessage(t *testing.T) { }) } } + +func TestAddCommitMessageTailer(t *testing.T) { + assert.Equal(t, "\n\nTest-tailer: TestValue", AddCommitMessageTailer("", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\n", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nTest-tailer: TestValue", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nTest-tailer: TestValue\n", AddCommitMessageTailer("title\n\nTest-tailer: TestValue\n", "Test-tailer", "TestValue")) + + assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTailer("title\n\nTest-tailer: v1", "Test-tailer", "v2")) + assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTailer("title\n\nTest-tailer: v1\n", "Test-tailer", "v2")) +} From 63640567b5acf3b6a75ec418e6c126fda19609ce Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 27 May 2025 19:58:25 +0800 Subject: [PATCH 4/5] improve --- services/pull/merge.go | 36 +++++++++++++++++++++++++++++++++++ services/pull/merge_squash.go | 30 ----------------------------- services/pull/merge_test.go | 11 +++++++++++ 3 files changed, 47 insertions(+), 30 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 256db847ef39b..fd1c86d15c2b3 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -13,6 +13,7 @@ import ( "regexp" "strconv" "strings" + "unicode" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" @@ -161,6 +162,41 @@ func GetDefaultMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr return getMergeMessage(ctx, baseGitRepo, pr, mergeStyle, nil) } +func AddCommitMessageTailer(message, tailerKey, tailerValue string) string { + tailerLine := tailerKey + ": " + tailerValue + message = strings.ReplaceAll(message, "\r\n", "\n") + message = strings.ReplaceAll(message, "\r", "\n") + if strings.Contains(message, "\n"+tailerLine+"\n") || strings.HasSuffix(message, "\n"+tailerLine) { + return message + } + + if !strings.HasSuffix(message, "\n") { + message += "\n" + } + pos1 := strings.LastIndexByte(message[:len(message)-1], '\n') + pos2 := -1 + if pos1 != -1 { + pos2 = strings.IndexByte(message[pos1:], ':') + if pos2 != -1 { + pos2 += pos1 + } + } + var lastLineKey string + if pos1 != -1 && pos2 != -1 { + lastLineKey = message[pos1+1 : pos2] + } + + isLikelyTailerLine := lastLineKey != "" && unicode.IsUpper(rune(lastLineKey[0])) + for i := 0; isLikelyTailerLine && i < len(lastLineKey); i++ { + r := rune(lastLineKey[i]) + isLikelyTailerLine = unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' + } + if !strings.HasSuffix(message, "\n\n") && !isLikelyTailerLine { + message += "\n" + } + return message + tailerLine +} + // ErrInvalidMergeStyle represents an error if merging with disabled merge strategy type ErrInvalidMergeStyle struct { ID int64 diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index 7042088b079c1..119b28736c031 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -5,8 +5,6 @@ package pull import ( "fmt" - "strings" - "unicode" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -52,34 +50,6 @@ func getAuthorSignatureSquash(ctx *mergeContext) (*git.Signature, error) { return ctx.pr.Issue.Poster.NewGitSig(), nil } -func AddCommitMessageTailer(message, tailerKey, tailerValue string) string { - tailerLine := tailerKey + ": " + tailerValue - if strings.Contains(message, "\n"+tailerLine+"\n") || strings.HasSuffix(message, "\n"+tailerLine) { - return message - } - - if !strings.HasSuffix(message, "\n") { - message += "\n" - } - pos1 := strings.LastIndexByte(message[:len(message)-1], '\n') - pos2 := -1 - if pos1 != -1 { - pos2 = strings.IndexByte(message[pos1:], ':') - if pos2 != -1 { - pos2 += pos1 - } - } - var lastLine string - if pos1 != -1 && pos2 != -1 { - lastLine = message[pos1+1 : pos2+1] - } - lastLineIsTailerLine := lastLine != "" && unicode.IsUpper(rune(lastLine[0])) && strings.Contains(lastLine, "-") - if !strings.HasSuffix(message, "\n\n") && !lastLineIsTailerLine { - message += "\n" - } - return message + tailerLine -} - // doMergeStyleSquash squashes the tracking branch on the current HEAD (=base) func doMergeStyleSquash(ctx *mergeContext, message string) error { sig, err := getAuthorSignatureSquash(ctx) diff --git a/services/pull/merge_test.go b/services/pull/merge_test.go index 3630ee92a5fdd..8baeaa3e2a969 100644 --- a/services/pull/merge_test.go +++ b/services/pull/merge_test.go @@ -67,13 +67,24 @@ func Test_expandDefaultMergeMessage(t *testing.T) { } func TestAddCommitMessageTailer(t *testing.T) { + // add tailer for empty message assert.Equal(t, "\n\nTest-tailer: TestValue", AddCommitMessageTailer("", "Test-tailer", "TestValue")) + + // add tailer for message without newlines assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nnot tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nnot tailer: xxx", "Test-tailer", "TestValue")) + + // add tailer for message with one EOL assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n", "Test-tailer", "TestValue")) + + // add tailer for message with two EOLs assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\n", "Test-tailer", "TestValue")) + + // add tailer for message with existing tailer (won't duplicate) assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nTest-tailer: TestValue", "Test-tailer", "TestValue")) assert.Equal(t, "title\n\nTest-tailer: TestValue\n", AddCommitMessageTailer("title\n\nTest-tailer: TestValue\n", "Test-tailer", "TestValue")) + // add tailer for message with existing tailer and different value (will append) assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTailer("title\n\nTest-tailer: v1", "Test-tailer", "v2")) assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTailer("title\n\nTest-tailer: v1\n", "Test-tailer", "v2")) } From d2965203abedd02cc015ff7f14fd1fa8993e0926 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 28 May 2025 00:58:31 +0800 Subject: [PATCH 5/5] improve --- services/pull/merge.go | 2 +- services/pull/merge_test.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index fd1c86d15c2b3..829d4b7ee1fee 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -186,7 +186,7 @@ func AddCommitMessageTailer(message, tailerKey, tailerValue string) string { lastLineKey = message[pos1+1 : pos2] } - isLikelyTailerLine := lastLineKey != "" && unicode.IsUpper(rune(lastLineKey[0])) + isLikelyTailerLine := lastLineKey != "" && unicode.IsUpper(rune(lastLineKey[0])) && strings.Contains(message, "-") for i := 0; isLikelyTailerLine && i < len(lastLineKey); i++ { r := rune(lastLineKey[i]) isLikelyTailerLine = unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' diff --git a/services/pull/merge_test.go b/services/pull/merge_test.go index 8baeaa3e2a969..91abeb9d9c169 100644 --- a/services/pull/merge_test.go +++ b/services/pull/merge_test.go @@ -72,7 +72,9 @@ func TestAddCommitMessageTailer(t *testing.T) { // add tailer for message without newlines assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title", "Test-tailer", "TestValue")) - assert.Equal(t, "title\n\nnot tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nnot tailer: xxx", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nNot tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nNot tailer: xxx", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nNotTailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nNotTailer: xxx", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nnot-tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nnot-tailer: xxx", "Test-tailer", "TestValue")) // add tailer for message with one EOL assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n", "Test-tailer", "TestValue"))