diff --git a/modules/git/commit.go b/modules/git/commit.go index e66a33ef98..b98d36d946 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -37,6 +37,10 @@ type CommitSignature struct { // Message returns the commit message. Same as retrieving CommitMessage directly. func (c *Commit) Message() string { + // FIXME: GIT-COMMIT-MESSAGE-ENCODING: this logic is not right + // * When need to use commit message in templates/database, it should be valid UTF-8 + // * When need to get the original commit message, it should just use "c.CommitMessage" + // It's not easy to refactor at the moment, many templates need to be updated and tested return c.CommitMessage } diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 1e2486f5f1..e034731e5c 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -7,7 +7,6 @@ import ( gocontext "context" "encoding/csv" "errors" - "fmt" "io" "net/http" "net/url" @@ -426,6 +425,36 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { return compareInfo } +func prepareNewPullRequestTitleContent(ci *git_service.CompareInfo, commits []*git_model.SignCommitWithStatuses) (title, content string) { + title = ci.HeadRef.ShortName() + + if len(commits) > 0 { + // the "commits" are from "ShowPrettyFormatLogToList", which is ordered from newest to oldest, here take the oldest one + c := commits[len(commits)-1] + title = strings.TrimSpace(c.UserCommit.Summary()) + } + + if len(commits) == 1 { + // FIXME: GIT-COMMIT-MESSAGE-ENCODING: try to convert the encoding for commit message explicitly, ideally it should be done by a git commit struct method + c := commits[0] + _, content, _ = strings.Cut(strings.TrimSpace(c.UserCommit.CommitMessage), "\n") + content = strings.TrimSpace(content) + content = string(charset.ToUTF8([]byte(content), charset.ConvertOpts{})) + } + + var titleTrailer string + // TODO: 255 doesn't seem to be a good limit for title, just keep the old behavior + title, titleTrailer = util.EllipsisDisplayStringX(title, 255) + if titleTrailer != "" { + if content != "" { + content = titleTrailer + "\n\n" + content + } else { + content = titleTrailer + "\n" + } + } + return title, content +} + // PrepareCompareDiff renders compare diff page func PrepareCompareDiff( ctx *context.Context, @@ -539,30 +568,7 @@ func PrepareCompareDiff( ctx.Data["Commits"] = commits ctx.Data["CommitCount"] = len(commits) - title := ci.HeadRef.ShortName() - if len(commits) == 1 { - c := commits[0] - title = strings.TrimSpace(c.UserCommit.Summary()) - - body := strings.Split(strings.TrimSpace(c.UserCommit.Message()), "\n") - if len(body) > 1 { - ctx.Data["content"] = strings.Join(body[1:], "\n") - } - } - - if len(title) > 255 { - var trailer string - title, trailer = util.EllipsisDisplayStringX(title, 255) - if len(trailer) > 0 { - if ctx.Data["content"] != nil { - ctx.Data["content"] = fmt.Sprintf("%s\n\n%s", trailer, ctx.Data["content"]) - } else { - ctx.Data["content"] = trailer + "\n" - } - } - } - - ctx.Data["title"] = title + ctx.Data["title"], ctx.Data["content"] = prepareNewPullRequestTitleContent(ci, commits) ctx.Data["Username"] = ci.HeadRepo.OwnerName ctx.Data["Reponame"] = ci.HeadRepo.Name diff --git a/routers/web/repo/compare_test.go b/routers/web/repo/compare_test.go index 61472dc71e..700aba8821 100644 --- a/routers/web/repo/compare_test.go +++ b/routers/web/repo/compare_test.go @@ -4,9 +4,16 @@ package repo import ( + "strings" "testing" + "unicode/utf8" + asymkey_model "code.gitea.io/gitea/models/asymkey" + git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + git_service "code.gitea.io/gitea/services/git" "code.gitea.io/gitea/services/gitdiff" "github.com/stretchr/testify/assert" @@ -38,3 +45,47 @@ func TestAttachCommentsToLines(t *testing.T) { assert.Equal(t, int64(300), section.Lines[1].Comments[0].ID) assert.Equal(t, int64(301), section.Lines[1].Comments[1].ID) } + +func TestNewPullRequestTitleContent(t *testing.T) { + ci := &git_service.CompareInfo{HeadRef: "refs/heads/head-branch"} + + mockCommit := func(msg string) *git_model.SignCommitWithStatuses { + return &git_model.SignCommitWithStatuses{ + SignCommit: &asymkey_model.SignCommit{ + UserCommit: &user_model.UserCommit{ + Commit: &git.Commit{ + CommitMessage: msg, + }, + }, + }, + } + } + + title, content := prepareNewPullRequestTitleContent(ci, nil) + assert.Equal(t, "head-branch", title) + assert.Empty(t, content) + + title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("title-only")}) + assert.Equal(t, "title-only", title) + assert.Empty(t, content) + + title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("title-" + strings.Repeat("a", 255))}) + assert.Equal(t, "title-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa…", title) + assert.Equal(t, "…aaaaaaaaa\n", content) + + title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("title\nbody")}) + assert.Equal(t, "title", title) + assert.Equal(t, "body", content) + + title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("a\xf0\xf0\xf0\nb\xf0\xf0\xf0")}) + assert.Equal(t, "a?", title) // FIXME: GIT-COMMIT-MESSAGE-ENCODING: "title" doesn't use the same charset converting logic as "content" + assert.Equal(t, "b"+string(utf8.RuneError)+string(utf8.RuneError), content) + + title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{ + // ordered from newest to oldest + mockCommit("title2\nbody2"), + mockCommit("title1\nbody1"), + }) + assert.Equal(t, "title1", title) + assert.Empty(t, content) +} diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index e4c8008c19..5475224750 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -6,7 +6,7 @@
{{ctx.AvatarUtils.Avatar .SignedUser 40}}
- diff --git a/web_src/js/features/common-button.ts b/web_src/js/features/common-button.ts index 72044d2d4e..4483060ade 100644 --- a/web_src/js/features/common-button.ts +++ b/web_src/js/features/common-button.ts @@ -2,6 +2,7 @@ import {POST} from '../modules/fetch.ts'; import {addDelegatedEventListener, hideElem, isElemVisible, showElem, toggleElem} from '../utils/dom.ts'; import {fomanticQuery} from '../modules/fomantic/base.ts'; import {camelize} from 'vue'; +import {applyAutoFocus} from './common-page.ts'; export function initGlobalButtonClickOnEnter(): void { addDelegatedEventListener(document, 'keypress', 'div.ui.button, span.ui.button', (el, e: KeyboardEvent) => { @@ -88,7 +89,7 @@ function onShowPanelClick(el: HTMLElement, e: MouseEvent) { const elems = el.classList.contains('toggle') ? toggleElem(sel) : showElem(sel); for (const elem of elems) { if (isElemVisible(elem as HTMLElement)) { - elem.querySelector('[autofocus]')?.focus(); + applyAutoFocus(elem); } } } diff --git a/web_src/js/features/common-page.ts b/web_src/js/features/common-page.ts index 5ecc271c0b..36af087089 100644 --- a/web_src/js/features/common-page.ts +++ b/web_src/js/features/common-page.ts @@ -116,12 +116,30 @@ function attachInputDirAuto(el: Partial) } } +function autoFocusEnd(el: HTMLInputElement | HTMLTextAreaElement) { + el.focus(); + el.setSelectionRange(el.value.length, el.value.length); +} + +export function applyAutoFocus(container: Element) { + // https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/autofocus + // "autofocus" behavior is defined by the standard: when a container (e.g.: dialog) becomes visible, focus the element with "autofocus" attribute + // Fomantic UI already supports it for its modal dialog, we need to cover more cases (e.g.: ".show-panel" button) + // Here is just a simple support, we don't expect more than one element that need "autofocus" appearing in the same container + container.querySelector('[autofocus]')?.focus(); + // Also, apply our autoFocusEnd behavior + // TODO: GLOBAL-INIT-MULTIPLE-FUNCTIONS: use "~=" operator in case we would extend the "data-global-init" to support more functions in the future. + const el = container.querySelector('[data-global-init~="autoFocusEnd"]'); + if (el) autoFocusEnd(el); +} + export function initGlobalInput() { registerGlobalSelectorFunc('input, textarea', attachInputDirAuto); - registerGlobalInitFunc('initInputAutoFocusEnd', (el: HTMLInputElement) => { - el.focus(); // expects only one such element on one page. If there are many, then the last one gets the focus. - el.setSelectionRange(el.value.length, el.value.length); - }); + + // autoFocusEnd is used for autofocus an input/textarea and move the cursor to the end of the text. + // It is useful for "New Issue"/"New PR" pages when the title is pre-filled with prefix text (e.g.: from template or commit message) + // The native "autofocus" isn't used because there is a delay between "focused (DOM rendering)" and "move cursor to end (our JS)", it causes flickers. + registerGlobalInitFunc('autoFocusEnd', autoFocusEnd); } /** diff --git a/web_src/js/modules/observer.ts b/web_src/js/modules/observer.ts index 1bbf304b27..1dbad1aed1 100644 --- a/web_src/js/modules/observer.ts +++ b/web_src/js/modules/observer.ts @@ -42,6 +42,7 @@ export function registerGlobalInitFunc(name: string, hand } function callGlobalInitFunc(el: HTMLElement) { + // TODO: GLOBAL-INIT-MULTIPLE-FUNCTIONS: maybe in the future we need to extend it to support multiple functions, for example: `data-global-init="func1 func2 func3"` const initFunc = el.getAttribute('data-global-init')!; const func = globalInitFuncs[initFunc]; if (!func) throw new Error(`Global init function "${initFunc}" not found`);