kyush/forgejo
1
0
Fork 0
forked from mirrors/forgejo
Code Pull requests Activity

fix: allow for merge message template with empty message (#9930)

Browse source 
Fixes #2691

This is changing the trimming of the merge message template to allow first empty line to be considered as empty message (also referred as title in frontend). 2 unit tests were added to check for proper behavior and possible edge case (simple template with only 2 lines, not trimming needed). Also a former unit test with 3 empty lines at the beginning of the template was changed.

The behavior with this patch is:
* splitting first `\n` to separate message from body
* trim remaining `\n` from body if `\n` was present or take template as is as message (no trim necessary as no `\n` is present in the template)

As this is an old issue the expected behavior might have changed from when the issue was triaged.

I additionally manually testes on the next.forgejo.org and locally built binary from this branch, I am attaching both screenshot of final result. The test PR can be seen at [https://v13.next.forgejo.org/kajika/merge_template_issue_2691/pulls/1](https://v13.next.forgejo.org/kajika/merge_template_issue_2691/pulls/1) and, in case the next repository is deletate the content of `.forgejo/default_merge_message/MERGE_TEMPLATE.md` is (first line is empty):
```

This is the description of the merge from "${HeadBranch}" to "${BaseBranch}"
```

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9930
Reviewed-by: oliverpool <oliverpool@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Corentin <corentin@ayo.tokyo>
Co-committed-by: Corentin <corentin@ayo.tokyo>
This commit is contained in:
Corentin 2025-12-10 19:57:19 +01:00 • committed by Gusted
commit 505ab87ae0
3 changed files with 127 additions and 58 deletions
Download patch file Download diff file Expand all files Collapse all files

58
services/pull/merge.go
View file

@ -87,6 +87,19 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue
reviewedOn := fmt.Sprintf("Reviewed-on: %s", issueURL)
reviewedBy := pr.GetApprovers(ctx)
body = fmt.Sprintf("%s\n%s", reviewedOn, reviewedBy)
// Squash merge has a different from other styles.
if mergeStyle == repo_model.MergeStyleSquash {
message = fmt.Sprintf("%s (%s%d)", pr.Issue.Title, issueReference, pr.Issue.Index)
} else if pr.BaseRepoID == pr.HeadRepoID {
message = fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch)
} else if pr.HeadRepo == nil {
message = fmt.Sprintf("Merge pull request '%s' (%s%d) from <deleted>:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch)
} else {
message = fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch)
}
if mergeStyle != "" {
commit, err := baseGitRepo.GetBranchCommit(pr.BaseRepo.DefaultBranch)
if err != nil {
@ -155,8 +168,7 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue
vars["ClosingIssues"] = ""
}
}
message, body = expandDefaultMergeMessage(templateContent, vars)
return message, body, nil
return expandDefaultMergeMessage(templateContent, vars, message, body)
}
}
@ -165,32 +177,30 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue
return "", "", nil
}
body = fmt.Sprintf("%s\n%s", reviewedOn, reviewedBy)
// Squash merge has a different from other styles.
if mergeStyle == repo_model.MergeStyleSquash {
return fmt.Sprintf("%s (%s%d)", pr.Issue.Title, issueReference, pr.Issue.Index), body, nil
}
if pr.BaseRepoID == pr.HeadRepoID {
return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch), body, nil
}
if pr.HeadRepo == nil {
return fmt.Sprintf("Merge pull request '%s' (%s%d) from <deleted>:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch), body, nil
}
return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch), body, nil
return message, body, nil
}
func expandDefaultMergeMessage(template string, vars map[string]string) (message, body string) {
message = strings.TrimSpace(template)
if splits := strings.SplitN(message, "\n", 2); len(splits) == 2 {
message = splits[0]
body = strings.TrimSpace(splits[1])
func expandDefaultMergeMessage(template string, vars map[string]string, message, body string) (finalMessage, finalBody string, err error) {
if template == "" {
return message, body, nil
}
mapping := func(s string) string { return vars[s] }
return os.Expand(message, mapping), os.Expand(body, mapping)
if splits := strings.SplitN(template, "\n", 2); len(splits) == 2 {
var templateTitle string
var templateBody string
if len(splits[0]) == 0 {
templateTitle = message
} else {
templateTitle = os.Expand(strings.TrimSpace(splits[0]), mapping)
}
if len(splits[1]) == 0 {
templateBody = body
} else {
templateBody = os.Expand(strings.TrimRightFunc(splits[1], unicode.IsSpace), mapping)
}
return templateTitle, templateBody, nil
}
return os.Expand(strings.TrimSpace(template), mapping), body, nil
}
// GetDefaultMergeMessage returns default message used when merging pull request

125
services/pull/merge_test.go
View file

@ -4,6 +4,7 @@
package pull
import (
"fmt"
"os"
"path"
"strings"
@ -27,6 +28,21 @@ func Test_expandDefaultMergeMessage(t *testing.T) {
template string
vars map[string]string
}
title := "PullRequestTitle"
description := "Pull\nRequest\nDescription\n"
vars := map[string]string{
"PullRequestTitle": title,
"PullRequestDescription": description,
}
defaultTitle := "default_title"
defaultBody := "default_body"
expectedTitle := fmt.Sprintf("Merge %s", title)
expectedDescription := fmt.Sprintf("Description: %s", description)
expectedDescriptionMultiLine := fmt.Sprintf("Description:\n\n%s", description)
expectedDescriptionMerged := fmt.Sprintf("\n\nMerge %s\n\nDescription:\n\n%s", title, description)
emptyString := ""
tests := []struct {
name string
args args
@ -34,47 +50,90 @@ func Test_expandDefaultMergeMessage(t *testing.T) {
wantBody string
}{
{
name: "single line",
args: args{
template: "Merge ${PullRequestTitle}",
vars: map[string]string{
"PullRequestTitle": "PullRequestTitle",
"PullRequestDescription": "Pull\nRequest\nDescription\n",
},
},
want: "Merge PullRequestTitle",
wantBody: "",
name: "empty template",
args: args{template: "", vars: vars},
want: defaultTitle,
wantBody: defaultBody,
},
{
name: "multiple lines",
args: args{
template: "Merge ${PullRequestTitle}\nDescription:\n\n${PullRequestDescription}\n",
vars: map[string]string{
"PullRequestTitle": "PullRequestTitle",
"PullRequestDescription": "Pull\nRequest\nDescription\n",
},
},
want: "Merge PullRequestTitle",
wantBody: "Description:\n\nPull\nRequest\nDescription\n",
name: "single line",
args: args{template: "Merge ${PullRequestTitle}", vars: vars},
want: expectedTitle,
wantBody: defaultBody,
},
{
name: "leading newlines",
args: args{
template: "\n\n\nMerge ${PullRequestTitle}\n\n\nDescription:\n\n${PullRequestDescription}\n",
vars: map[string]string{
"PullRequestTitle": "PullRequestTitle",
"PullRequestDescription": "Pull\nRequest\nDescription\n",
},
},
want: "Merge PullRequestTitle",
wantBody: "Description:\n\nPull\nRequest\nDescription\n",
name: "empty message (space)",
args: args{template: " ", vars: vars},
want: emptyString,
wantBody: defaultBody,
},
{
name: "empty message (with newline)",
args: args{template: " \n", vars: vars},
want: emptyString,
wantBody: defaultBody,
},
{
name: "single newline",
args: args{template: "\n", vars: vars},
want: defaultTitle,
wantBody: defaultBody,
},
{
name: "empty description (newline)",
args: args{template: "\n\n", vars: vars},
want: defaultTitle,
wantBody: emptyString,
},
{
name: "empty description (space)",
args: args{template: "\n ", vars: vars},
want: defaultTitle,
wantBody: emptyString,
},
{
name: "empty title and description (spaces)",
args: args{template: " \n ", vars: vars},
want: emptyString,
wantBody: emptyString,
},
{
name: "empty title and description (space and newline)",
args: args{template: " \n\n", vars: vars},
want: emptyString,
wantBody: emptyString,
},
{
name: "simple message and description",
args: args{template: "Merge ${PullRequestTitle}\nDescription: ${PullRequestDescription}", vars: vars},
want: expectedTitle,
wantBody: expectedDescription,
},
{
name: "multiple lines",
args: args{template: "Merge ${PullRequestTitle}\nDescription:\n\n${PullRequestDescription}\n", vars: vars},
want: expectedTitle,
wantBody: expectedDescriptionMultiLine,
},
{
name: "description only",
args: args{template: "\nDescription: ${PullRequestDescription}\n", vars: vars},
want: defaultTitle,
wantBody: expectedDescription,
},
{
name: "leading newlines",
args: args{template: "\n\n\nMerge ${PullRequestTitle}\n\nDescription:\n\n${PullRequestDescription}\n", vars: vars},
want: defaultTitle,
wantBody: expectedDescriptionMerged,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, got1 := expandDefaultMergeMessage(tt.args.template, tt.args.vars)
assert.Equalf(t, tt.want, got, "expandDefaultMergeMessage(%v, %v)", tt.args.template, tt.args.vars)
assert.Equalf(t, tt.wantBody, got1, "expandDefaultMergeMessage(%v, %v)", tt.args.template, tt.args.vars)
resultTitle, resultBody, err := expandDefaultMergeMessage(tt.args.template, tt.args.vars, defaultTitle, defaultBody)
require.NoError(t, err)
assert.Equalf(t, tt.want, resultTitle, "Wrong title for test '%s' -> expandDefaultMergeMessage(%q, %q)", tt.name, tt.args.template, tt.args.vars)
assert.Equalf(t, tt.wantBody, resultBody, "Wrong body for test '%s' -> expandDefaultMergeMessage(%q, %q)", tt.name, tt.args.template, tt.args.vars)
})
}
}

2
services/pull/pull_test.go
View file

@ -126,5 +126,5 @@ func TestPullRequest_GetDefaultMergeMessage_GlobalTemplate(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Default merge message template", mergeMessage)
assert.Equal(t, "This line was read from .forgejo/default_merge_message/MERGE_TEMPLATE.md", body)
assert.Equal(t, "\nThis line was read from .forgejo/default_merge_message/MERGE_TEMPLATE.md", body)
}