Compare commits

...
Author SHA1 Message Date
wxiaoguang da91800c1b add fixme comment 2026-05-31 19:25:43 +08:00
wxiaoguang 9392da4d64 fix fmt 2026-05-31 19:17:57 +08:00
Nicolas f99aa56c4f fix 2026-05-31 12:57:52 +02:00
f5d0e1633d fix(pull): preserve squash message trailers and additional commit messages
When a PR description already ends with git trailers (e.g. Issue: X,
Signed-off-by:), the co-author separator line (---------)  was still
inserted before the Co-authored-by lines, breaking the trailer block.
messageHasTrailers now skips the separator so co-authors are appended
directly into the existing trailer block.

In PR-description mode (PopulateSquashCommentWithCommitMessages=false),
commit messages beyond the oldest were silently dropped. They are now
appended as bullet points after the PR description, consistent with the
commit-message mode format.

The commit-message loop is extracted into formatSquashMergeCommitMessages.
Callers that want to skip the oldest commit pass a trimmed slice
(commits[:max(0, len(commits)-1)]) instead of a skipFirst bool flag.

Co-Authored-By: Nicolas <nicolas@bircks.eu>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-31 12:12:36 +02:00
2 changed files with 83 additions and 31 deletions
+55 -31
View File
@@ -4,6 +4,7 @@
package pull
import (
"bytes"
"context"
"errors"
"fmt"
@@ -831,45 +832,33 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
authors := make([]string, 0, len(commits))
stringBuilder := strings.Builder{}
// trailerBlockAtEnd tracks whether the message currently ends with a Git trailer block.
// When true, we skip the "---------" separator so Co-authored-by lines stay contiguous with it.
trailerBlockAtEnd := false
if !setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages {
// use PR's title and description as squash commit message
message := strings.TrimSpace(pr.Issue.Content)
messageHasTrailers := commitMessageTrailersPattern.MatchString(message)
stringBuilder.WriteString(message)
if stringBuilder.Len() > 0 {
// FIXME: is it right? why `len(commits)-1)`?
additionalCommitMessages := formatSquashMergeCommitMessages(commits[:max(0, len(commits)-1)])
if additionalCommitMessages != "" {
if stringBuilder.Len() > 0 {
stringBuilder.WriteString("\n\n")
}
// FIXME: is it right? what if `pr.Issue.Content` contains trailers?
stringBuilder.WriteString(additionalCommitMessages)
// appended bullets push the PR-description trailers (if any) out of the trailer-block position
} else if stringBuilder.Len() > 0 {
stringBuilder.WriteRune('\n')
if !commitMessageTrailersPattern.MatchString(message) {
// TODO: this trailer check doesn't work with the separator line added below for the co-authors
if !messageHasTrailers {
stringBuilder.WriteRune('\n')
}
trailerBlockAtEnd = messageHasTrailers
}
} else {
// use PR's commit messages as squash commit message
// commits list is in reverse chronological order
maxMsgSize := setting.Repository.PullRequest.DefaultMergeMessageSize
for _, commit := range slices.Backward(commits) {
msg := strings.TrimSpace(commit.MessageUTF8())
if msg == "" {
continue
}
// This format follows GitHub's squash commit message style,
// even if there are other "* " in the commit message body, they are written as-is.
// Maybe, ideally, we should indent those lines too.
_, _ = fmt.Fprintf(&stringBuilder, "* %s\n\n", msg)
if maxMsgSize > 0 && stringBuilder.Len() >= maxMsgSize {
tmp := stringBuilder.String()
wasValidUtf8 := utf8.ValidString(tmp)
tmp = tmp[:maxMsgSize] + "..."
if wasValidUtf8 {
// If the message was valid UTF-8 before truncation, ensure it remains valid after truncation
// For non-utf8 messages, we can't do much about it, end users should use utf-8 as much as possible
tmp = strings.ToValidUTF8(tmp, "")
}
stringBuilder.Reset()
stringBuilder.WriteString(tmp)
break
}
}
stringBuilder.WriteString(formatSquashMergeCommitMessages(commits))
}
// collect co-authors
@@ -911,8 +900,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
}
}
if stringBuilder.Len() > 0 && len(authors) > 0 {
// TODO: this separator line doesn't work with the trailer check (commitMessageTrailersPattern) above
if stringBuilder.Len() > 0 && len(authors) > 0 && !trailerBlockAtEnd {
stringBuilder.WriteString("---------\n\n")
}
@@ -925,6 +913,42 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
return stringBuilder.String()
}
func formatSquashMergeCommitMessages(commits []*git.Commit) string {
maxMsgSize := setting.Repository.PullRequest.DefaultMergeMessageSize
sb := &bytes.Buffer{}
// commits list is in reverse chronological order
for _, commit := range slices.Backward(commits) {
msg := strings.TrimSpace(commit.MessageUTF8())
if msg == "" {
continue
}
// This format follows GitHub's squash commit message style,
// even if there are other "* " in the commit message body, they are written as-is.
// Maybe, ideally, we should indent those lines too.
_, _ = fmt.Fprintf(sb, "* %s\n\n", msg)
if maxMsgSize > 0 && sb.Len() >= maxMsgSize {
break
}
}
buf := bytes.TrimSpace(sb.Bytes())
if maxMsgSize > 0 && len(buf) > maxMsgSize {
buf = buf[:maxMsgSize]
for {
r, sz := utf8.DecodeLastRune(buf)
if r == utf8.RuneError && sz == 1 {
buf = buf[:len(buf)-1]
continue
}
break
}
buf = append(buf, '.', '.', '.')
}
buf = append(buf, '\n', '\n')
return util.UnsafeBytesToString(buf)
}
// GetIssuesAllCommitStatus returns a map of issue ID to a list of all statuses for the most recent commit as well as a map of issue ID to only the commit's latest status
func GetIssuesAllCommitStatus(ctx context.Context, issues issues_model.IssueList) (map[int64][]*git_model.CommitStatus, map[int64]*git_model.CommitStatus, error) {
if err := issues.LoadPullRequests(ctx); err != nil {
+28
View File
@@ -11,7 +11,10 @@ import (
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unit"
"gitea.dev/models/unittest"
"gitea.dev/modules/git"
"gitea.dev/modules/gitrepo"
"gitea.dev/modules/setting"
"gitea.dev/modules/test"
"github.com/stretchr/testify/assert"
)
@@ -35,6 +38,31 @@ func TestPullRequest_CommitMessageTrailersPattern(t *testing.T) {
assert.True(t, commitMessageTrailersPattern.MatchString("Folded value.\n\nFolded-trailer: This is\n a folded\n trailer value\nOther-Trailer: Value"))
}
func TestPullRequest_FormatSquashMergeCommitMessages(t *testing.T) {
oldest := &git.Commit{CommitMessage: git.CommitMessage{MessageRaw: "commit msg 1"}}
newest := &git.Commit{CommitMessage: git.CommitMessage{MessageRaw: "commit msg 2\n\nCommit description."}}
defer test.MockVariableValue(&setting.Repository.PullRequest.DefaultMergeMessageSize, 0)()
// all commits
assert.Equal(t, "* commit msg 1\n\n* commit msg 2\n\nCommit description.\n\n",
formatSquashMergeCommitMessages([]*git.Commit{newest, oldest}))
// PR-description mode: pass all-but-oldest so the oldest is not duplicated
assert.Equal(t, "* commit msg 2\n\nCommit description.\n\n",
formatSquashMergeCommitMessages([]*git.Commit{newest}))
utf8Msg := &git.Commit{CommitMessage: git.CommitMessage{MessageRaw: "🌞"}}
setting.Repository.PullRequest.DefaultMergeMessageSize = 3
assert.Equal(t, "* ...\n\n", formatSquashMergeCommitMessages([]*git.Commit{utf8Msg}))
setting.Repository.PullRequest.DefaultMergeMessageSize = 4
assert.Equal(t, "* ...\n\n", formatSquashMergeCommitMessages([]*git.Commit{utf8Msg}))
setting.Repository.PullRequest.DefaultMergeMessageSize = 5
assert.Equal(t, "* ...\n\n", formatSquashMergeCommitMessages([]*git.Commit{utf8Msg}))
setting.Repository.PullRequest.DefaultMergeMessageSize = 6
assert.Equal(t, "* 🌞\n\n", formatSquashMergeCommitMessages([]*git.Commit{utf8Msg}))
}
func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})