diff --git a/services/pull/merge.go b/services/pull/merge.go index 3f07c3df9b..f69f8a87b4 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -14,6 +14,7 @@ import ( "regexp" "strconv" "strings" + "unicode" "forgejo.org/models" "forgejo.org/models/db" @@ -169,6 +170,41 @@ func GetDefaultMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr return getMergeMessage(ctx, baseGitRepo, pr, mergeStyle, nil) } +func AddCommitMessageTrailer(message, tailerKey, tailerValue string) string { + trailerLine := tailerKey + ": " + tailerValue + message = strings.ReplaceAll(message, "\r\n", "\n") + message = strings.ReplaceAll(message, "\r", "\n") + if strings.Contains(message, "\n"+trailerLine+"\n") || strings.HasSuffix(message, "\n"+trailerLine) { + return message + } + + if !strings.HasSuffix(message, "\n") { + message += "\n" + } + lastNewLine := strings.LastIndexByte(message[:len(message)-1], '\n') + keyEnd := -1 + if lastNewLine != -1 { + keyEnd = strings.IndexByte(message[lastNewLine:], ':') + if keyEnd != -1 { + keyEnd += lastNewLine + } + } + var lastLineKey string + if lastNewLine != -1 && keyEnd != -1 { + lastLineKey = message[lastNewLine+1 : keyEnd] + } + + isLikelyTrailerLine := lastLineKey != "" && unicode.IsUpper(rune(lastLineKey[0])) && strings.Contains(message, "-") + for i := 0; isLikelyTrailerLine && i < len(lastLineKey); i++ { + r := rune(lastLineKey[i]) + isLikelyTrailerLine = unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' + } + if !strings.HasSuffix(message, "\n\n") && !isLikelyTrailerLine { + message += "\n" + } + return message + trailerLine +} + // Merge merges pull request to base repository. // Caller should check PR is ready to be merged (review and status checks) func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, wasAutoMerged bool) error { diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index 1c6f734a25..f655224c5e 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -5,7 +5,6 @@ package pull import ( "fmt" - "strings" repo_model "forgejo.org/models/repo" user_model "forgejo.org/models/user" @@ -67,10 +66,8 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error { if setting.Repository.PullRequest.AddCoCommitterTrailers && ctx.committer.String() != sig.String() { // add trailer - if !strings.Contains(message, fmt.Sprintf("Co-authored-by: %s", sig.String())) { - message += fmt.Sprintf("\nCo-authored-by: %s", sig.String()) - } - message += fmt.Sprintf("\nCo-committed-by: %s\n", sig.String()) + message = AddCommitMessageTrailer(message, "Co-authored-by", sig.String()) + message = AddCommitMessageTrailer(message, "Co-committed-by", sig.String()) // FIXME: this one should be removed, it is not really used or widely used } cmdCommit := git.NewCommand(ctx, "commit"). AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). diff --git a/services/pull/merge_test.go b/services/pull/merge_test.go index 6df6f55d46..2a26759956 100644 --- a/services/pull/merge_test.go +++ b/services/pull/merge_test.go @@ -65,3 +65,28 @@ func Test_expandDefaultMergeMessage(t *testing.T) { }) } } + +func TestAddCommitMessageTailer(t *testing.T) { + // add tailer for empty message + assert.Equal(t, "\n\nTest-tailer: TestValue", AddCommitMessageTrailer("", "Test-tailer", "TestValue")) + + // add tailer for message without newlines + assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nNot tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nNot tailer: xxx", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nNotTailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nNotTailer: xxx", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nnot-tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nnot-tailer: xxx", "Test-tailer", "TestValue")) + + // add tailer for message with one EOL + assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n", "Test-tailer", "TestValue")) + + // add tailer for message with two EOLs + assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\n", "Test-tailer", "TestValue")) + + // add tailer for message with existing tailer (won't duplicate) + assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nTest-tailer: TestValue", "Test-tailer", "TestValue")) + assert.Equal(t, "title\n\nTest-tailer: TestValue\n", AddCommitMessageTrailer("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", AddCommitMessageTrailer("title\n\nTest-tailer: v1", "Test-tailer", "v2")) + assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTrailer("title\n\nTest-tailer: v1\n", "Test-tailer", "v2")) +}