diff options
| -rw-r--r-- | options/locale/locale_fr-FR.ini | 2 | ||||
| -rw-r--r-- | routers/web/repo/pull.go | 143 | ||||
| -rw-r--r-- | routers/web/web.go | 5 | ||||
| -rw-r--r-- | templates/repo/diff/box.tmpl | 2 | ||||
| -rw-r--r-- | tests/integration/pull_diff_test.go | 4 | ||||
| -rw-r--r-- | web_src/js/components/DiffCommitSelector.vue | 43 |
6 files changed, 91 insertions, 108 deletions
diff --git a/options/locale/locale_fr-FR.ini b/options/locale/locale_fr-FR.ini index 35bd0b897c..6f0008af67 100644 --- a/options/locale/locale_fr-FR.ini +++ b/options/locale/locale_fr-FR.ini @@ -269,6 +269,7 @@ path=Emplacement sqlite_helper=Chemin d'accès pour la base de données SQLite3.<br>Entrer un chemin absolu si vous exécutez Gitea en tant que service. reinstall_error=Vous essayez d'installer dans une base de données Gitea existante reinstall_confirm_message=La réinstallation avec une base de données Gitea existante peut causer plusieurs problèmes. Dans la plupart des cas, vous devriez utiliser votre "app.ini" existant pour exécuter Gitea. Si vous savez ce que vous faites, confirmez ce qui suit : +reinstall_confirm_check_1=Les données chiffrées par la valeur de SECRET_KEY dans app.ini peuvent être invalidés, empêchant les utilisateurs de se connecter via 2FA/OTP et les miroirs de se synchroniser. En cochant cette case, vous confirmez que le fichier app.ini actuel contient la bonne clé SECRET_KEY. reinstall_confirm_check_2=Les dépôts et les paramètres peuvent avoir besoin d'être re-synchronisés. En cochant cette case, vous confirmez que vous resynchroniserez manuellement les liens des dépôts et du fichier authorized_keys. Vous confirmez que vous allez vous assurer que les paramètres du dépôt et du miroir sont corrects. reinstall_confirm_check_3=Vous confirmez : vous êtes absolument certain que ce Gitea fonctionne avec le bon emplacement de app.ini et vous êtes certain de devoir réinstaller. Vous confirmez également que vous avez pris connaissance des risques exposés ci-dessus. err_empty_db_path=Le chemin de la base de données SQLite3 ne peut être vide. @@ -3252,6 +3253,7 @@ auths.oauth2_required_claim_name_helper=Définissez ce nom pour restreindre la c auths.oauth2_required_claim_value=Valeur de réclamation requise auths.oauth2_required_claim_value_helper=Restreindre la connexion depuis cette source aux utilisateurs ayant réclamé cette valeur. auths.oauth2_group_claim_name=Réclamer le nom fournissant les noms de groupe pour cette source. (facultatif) +auths.oauth2_full_name_claim_name=Nom complet réclamé. (Optionnel. Si défini, le nom complet de l’utilisateur sera toujours synchronisé avec cette réclamation) auths.oauth2_ssh_public_key_claim_name=Nom réclamé de la clé publique SSH auths.oauth2_admin_group=Valeur de réclamation de groupe pour les administrateurs. (Optionnel, nécessite un nom de réclamation) auths.oauth2_restricted_group=Valeur de réclamation de groupe pour les utilisateurs restreints. (Optionnel, nécessite un nom de réclamation) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 9f71ab4632..bc58efeb6f 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -643,8 +643,17 @@ func ViewPullCommits(ctx *context.Context) { ctx.HTML(http.StatusOK, tplPullCommits) } +func indexCommit(commits []*git.Commit, commitID string) *git.Commit { + for i := range commits { + if commits[i].ID.String() == commitID { + return commits[i] + } + } + return nil +} + // ViewPullFiles render pull request changed files list page -func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommit string, willShowSpecifiedCommitRange, willShowSpecifiedCommit bool) { +func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) { ctx.Data["PageIsPullList"] = true ctx.Data["PageIsPullFiles"] = true @@ -654,11 +663,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } pull := issue.PullRequest - var ( - startCommitID string - endCommitID string - gitRepo = ctx.Repo.GitRepo - ) + gitRepo := ctx.Repo.GitRepo prInfo := preparePullViewPullInfo(ctx, issue) if ctx.Written() { @@ -668,77 +673,68 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi return } - // Validate the given commit sha to show (if any passed) - if willShowSpecifiedCommit || willShowSpecifiedCommitRange { - foundStartCommit := len(specifiedStartCommit) == 0 - foundEndCommit := len(specifiedEndCommit) == 0 - - if !(foundStartCommit && foundEndCommit) { - for _, commit := range prInfo.Commits { - if commit.ID.String() == specifiedStartCommit { - foundStartCommit = true - } - if commit.ID.String() == specifiedEndCommit { - foundEndCommit = true - } - - if foundStartCommit && foundEndCommit { - break - } - } - } - - if !(foundStartCommit && foundEndCommit) { - ctx.NotFound(nil) - return - } - } - - if ctx.Written() { - return - } - headCommitID, err := gitRepo.GetRefCommitID(pull.GetGitHeadRefName()) if err != nil { ctx.ServerError("GetRefCommitID", err) return } - ctx.Data["IsShowingOnlySingleCommit"] = willShowSpecifiedCommit + isSingleCommit := beforeCommitID == "" && afterCommitID != "" + ctx.Data["IsShowingOnlySingleCommit"] = isSingleCommit + isShowAllCommits := (beforeCommitID == "" || beforeCommitID == prInfo.MergeBase) && (afterCommitID == "" || afterCommitID == headCommitID) + ctx.Data["IsShowingAllCommits"] = isShowAllCommits - if willShowSpecifiedCommit || willShowSpecifiedCommitRange { - if len(specifiedEndCommit) > 0 { - endCommitID = specifiedEndCommit - } else { - endCommitID = headCommitID - } - if len(specifiedStartCommit) > 0 { - startCommitID = specifiedStartCommit + if afterCommitID == "" || afterCommitID == headCommitID { + afterCommitID = headCommitID + } + afterCommit := indexCommit(prInfo.Commits, afterCommitID) + if afterCommit == nil { + ctx.HTTPError(http.StatusBadRequest, "after commit not found in PR commits") + return + } + + var beforeCommit *git.Commit + if !isSingleCommit { + if beforeCommitID == "" || beforeCommitID == prInfo.MergeBase { + beforeCommitID = prInfo.MergeBase + // mergebase commit is not in the list of the pull request commits + beforeCommit, err = gitRepo.GetCommit(beforeCommitID) + if err != nil { + ctx.ServerError("GetCommit", err) + return + } } else { - startCommitID = prInfo.MergeBase + beforeCommit = indexCommit(prInfo.Commits, beforeCommitID) + if beforeCommit == nil { + ctx.HTTPError(http.StatusBadRequest, "before commit not found in PR commits") + return + } } - ctx.Data["IsShowingAllCommits"] = false } else { - endCommitID = headCommitID - startCommitID = prInfo.MergeBase - ctx.Data["IsShowingAllCommits"] = true + beforeCommit, err = afterCommit.Parent(0) + if err != nil { + ctx.ServerError("Parent", err) + return + } + beforeCommitID = beforeCommit.ID.String() } ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name - ctx.Data["AfterCommitID"] = endCommitID - ctx.Data["BeforeCommitID"] = startCommitID - - fileOnly := ctx.FormBool("file-only") + ctx.Data["MergeBase"] = prInfo.MergeBase + ctx.Data["AfterCommitID"] = afterCommitID + ctx.Data["BeforeCommitID"] = beforeCommitID maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles files := ctx.FormStrings("files") + fileOnly := ctx.FormBool("file-only") if fileOnly && (len(files) == 2 || len(files) == 1) { maxLines, maxFiles = -1, -1 } diffOptions := &gitdiff.DiffOptions{ - AfterCommitID: endCommitID, + BeforeCommitID: beforeCommitID, + AfterCommitID: afterCommitID, SkipTo: ctx.FormString("skip-to"), MaxLines: maxLines, MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, @@ -746,10 +742,6 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), } - if !willShowSpecifiedCommit { - diffOptions.BeforeCommitID = startCommitID - } - diff, err := gitdiff.GetDiffForRender(ctx, ctx.Repo.RepoLink, gitRepo, diffOptions, files...) if err != nil { ctx.ServerError("GetDiff", err) @@ -761,7 +753,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi // as the viewed information is designed to be loaded only on latest PR // diff and if you're signed in. var reviewState *pull_model.ReviewState - if ctx.IsSigned && !willShowSpecifiedCommit && !willShowSpecifiedCommitRange { + if ctx.IsSigned && isShowAllCommits { reviewState, err = gitdiff.SyncUserSpecificDiff(ctx, ctx.Doer.ID, pull, gitRepo, diff, diffOptions) if err != nil { ctx.ServerError("SyncUserSpecificDiff", err) @@ -769,7 +761,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } } - diffShortStat, err := gitdiff.GetDiffShortStat(ctx.Repo.GitRepo, startCommitID, endCommitID) + diffShortStat, err := gitdiff.GetDiffShortStat(ctx.Repo.GitRepo, beforeCommitID, afterCommitID) if err != nil { ctx.ServerError("GetDiffShortStat", err) return @@ -816,7 +808,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi if !fileOnly { // note: use mergeBase is set to false because we already have the merge base from the pull request info - diffTree, err := gitdiff.GetDiffTree(ctx, gitRepo, false, startCommitID, endCommitID) + diffTree, err := gitdiff.GetDiffTree(ctx, gitRepo, false, beforeCommitID, afterCommitID) if err != nil { ctx.ServerError("GetDiffTree", err) return @@ -836,17 +828,6 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi ctx.Data["Diff"] = diff ctx.Data["DiffNotAvailable"] = diffShortStat.NumFiles == 0 - baseCommit, err := ctx.Repo.GitRepo.GetCommit(startCommitID) - if err != nil { - ctx.ServerError("GetCommit", err) - return - } - commit, err := gitRepo.GetCommit(endCommitID) - if err != nil { - ctx.ServerError("GetCommit", err) - return - } - if ctx.IsSigned && ctx.Doer != nil { if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil { ctx.ServerError("CanMarkConversation", err) @@ -854,7 +835,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } } - setCompareContext(ctx, baseCommit, commit, ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) + setCompareContext(ctx, beforeCommit, afterCommit, ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) assigneeUsers, err := repo_model.GetRepoAssignees(ctx, ctx.Repo.Repository) if err != nil { @@ -901,7 +882,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool { return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee) } - if !willShowSpecifiedCommit && !willShowSpecifiedCommitRange && pull.Flow == issues_model.PullRequestFlowGithub { + if isShowAllCommits && pull.Flow == issues_model.PullRequestFlowGithub { if err := pull.LoadHeadRepo(ctx); err != nil { ctx.ServerError("LoadHeadRepo", err) return @@ -930,19 +911,17 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } func ViewPullFilesForSingleCommit(ctx *context.Context) { - viewPullFiles(ctx, "", ctx.PathParam("sha"), true, true) + // it doesn't support showing files from mergebase to the special commit + // otherwise it will be ambiguous + viewPullFiles(ctx, "", ctx.PathParam("sha")) } func ViewPullFilesForRange(ctx *context.Context) { - viewPullFiles(ctx, ctx.PathParam("shaFrom"), ctx.PathParam("shaTo"), true, false) -} - -func ViewPullFilesStartingFromCommit(ctx *context.Context) { - viewPullFiles(ctx, "", ctx.PathParam("sha"), true, false) + viewPullFiles(ctx, ctx.PathParam("shaFrom"), ctx.PathParam("shaTo")) } func ViewPullFilesForAllCommitsOfPr(ctx *context.Context) { - viewPullFiles(ctx, "", "", false, false) + viewPullFiles(ctx, "", "") } // UpdatePullRequest merge PR's baseBranch into headBranch diff --git a/routers/web/web.go b/routers/web/web.go index f8612db504..09be0c3904 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1531,7 +1531,7 @@ func registerWebRoutes(m *web.Router) { m.Group("/commits", func() { m.Get("", repo.SetWhitespaceBehavior, repo.GetPullDiffStats, repo.ViewPullCommits) m.Get("/list", repo.GetPullCommits) - m.Get("/{sha:[a-f0-9]{7,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForSingleCommit) + m.Get("/{sha:[a-f0-9]{7,64}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForSingleCommit) }) m.Post("/merge", context.RepoMustNotBeArchived(), web.Bind(forms.MergePullRequestForm{}), repo.MergePullRequest) m.Post("/cancel_auto_merge", context.RepoMustNotBeArchived(), repo.CancelAutoMergePullRequest) @@ -1540,8 +1540,7 @@ func registerWebRoutes(m *web.Router) { m.Post("/cleanup", context.RepoMustNotBeArchived(), repo.CleanUpPullRequest) m.Group("/files", func() { m.Get("", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForAllCommitsOfPr) - m.Get("/{sha:[a-f0-9]{7,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesStartingFromCommit) - m.Get("/{shaFrom:[a-f0-9]{7,40}}..{shaTo:[a-f0-9]{7,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForRange) + m.Get("/{shaFrom:[a-f0-9]{7,64}}..{shaTo:[a-f0-9]{7,64}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForRange) m.Group("/reviews", func() { m.Get("/new_comment", repo.RenderNewCodeCommentForm) m.Post("/comments", web.Bind(forms.CodeCommentForm{}), repo.SetShowOutdatedComments, repo.CreateCodeComment) diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 22abf9a219..e4d1efac57 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -35,7 +35,7 @@ {{template "repo/diff/whitespace_dropdown" .}} {{template "repo/diff/options_dropdown" .}} {{if .PageIsPullFiles}} - <div id="diff-commit-select" data-issuelink="{{$.Issue.Link}}" data-queryparams="?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}" data-filter_changes_by_commit="{{ctx.Locale.Tr "repo.pulls.filter_changes_by_commit"}}"> + <div id="diff-commit-select" data-merge-base="{{.MergeBase}}" data-issuelink="{{$.Issue.Link}}" data-queryparams="?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}" data-filter_changes_by_commit="{{ctx.Locale.Tr "repo.pulls.filter_changes_by_commit"}}"> {{/* the following will be replaced by vue component, but this avoids any loading artifacts till the vue component is initialized */}} <div class="ui jump dropdown tiny basic button custom"> {{svg "octicon-git-commit"}} diff --git a/tests/integration/pull_diff_test.go b/tests/integration/pull_diff_test.go index 5411250935..0b286fd2b2 100644 --- a/tests/integration/pull_diff_test.go +++ b/tests/integration/pull_diff_test.go @@ -25,10 +25,6 @@ func TestPullDiff_CommitRangePRDiff(t *testing.T) { doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files/4ca8bcaf27e28504df7bf996819665986b01c847..23576dd018294e476c06e569b6b0f170d0558705", true, []string{"test2.txt", "test3.txt", "test4.txt"}) } -func TestPullDiff_StartingFromBaseToCommitPRDiff(t *testing.T) { - doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files/c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2", true, []string{"test1.txt", "test2.txt", "test3.txt"}) -} - func doTestPRDiff(t *testing.T, prDiffURL string, reviewBtnDisabled bool, expectedFilenames []string) { defer tests.PrepareTestEnv(t)() diff --git a/web_src/js/components/DiffCommitSelector.vue b/web_src/js/components/DiffCommitSelector.vue index 4b18694bd2..e9aa3c6744 100644 --- a/web_src/js/components/DiffCommitSelector.vue +++ b/web_src/js/components/DiffCommitSelector.vue @@ -32,6 +32,7 @@ export default defineComponent({ locale: { filter_changes_by_commit: el.getAttribute('data-filter_changes_by_commit'), } as Record<string, string>, + mergeBase: el.getAttribute('data-merge-base'), commits: [] as Array<Commit>, hoverActivated: false, lastReviewCommitSha: '', @@ -176,32 +177,38 @@ export default defineComponent({ } }, /** - * When a commit is clicked with shift this enables the range - * selection. Second click (with shift) defines the end of the - * range. This opens the diff of this range - * Exception: first commit is the first commit of this PR. Then - * the diff from beginning of PR up to the second clicked commit is - * opened + * When a commit is clicked while holding Shift, it enables range selection. + * - The range selection is a half-open, half-closed range, meaning it excludes the start commit but includes the end commit. + * - The start of the commit range is always the previous commit of the first clicked commit. + * - If the first commit in the list is clicked, the mergeBase will be used as the start of the range instead. + * - The second Shift-click defines the end of the range. + * - Once both are selected, the diff view for the selected commit range will open. */ commitClickedShift(commit: Commit) { this.hoverActivated = !this.hoverActivated; commit.selected = true; // Second click -> determine our range and open links accordingly if (!this.hoverActivated) { + // since at least one commit is selected, we can determine the range // find all selected commits and generate a link - if (this.commits[0].selected) { - // first commit is selected - generate a short url with only target sha - const lastCommitIdx = this.commits.findLastIndex((x) => x.selected); - if (lastCommitIdx === this.commits.length - 1) { - // user selected all commits - just show the normal diff page - window.location.assign(`${this.issueLink}/files${this.queryParams}`); - } else { - window.location.assign(`${this.issueLink}/files/${this.commits[lastCommitIdx].id}${this.queryParams}`); - } + const firstSelected = this.commits.findIndex((x) => x.selected); + const lastSelected = this.commits.findLastIndex((x) => x.selected); + let beforeCommitID: string; + if (firstSelected === 0) { + beforeCommitID = this.mergeBase; } else { - const start = this.commits[this.commits.findIndex((x) => x.selected) - 1].id; - const end = this.commits.findLast((x) => x.selected).id; - window.location.assign(`${this.issueLink}/files/${start}..${end}${this.queryParams}`); + beforeCommitID = this.commits[firstSelected - 1].id; + } + const afterCommitID = this.commits[lastSelected].id; + + if (firstSelected === lastSelected) { + // if the start and end are the same, we show this single commit + window.location.assign(`${this.issueLink}/commits/${afterCommitID}${this.queryParams}`); + } else if (beforeCommitID === this.mergeBase && afterCommitID === this.commits.at(-1).id) { + // if the first commit is selected and the last commit is selected, we show all commits + window.location.assign(`${this.issueLink}/files${this.queryParams}`); + } else { + window.location.assign(`${this.issueLink}/files/${beforeCommitID}..${afterCommitID}${this.queryParams}`); } } }, |