Add commits dropdown in PR files view and allow commit by commit review

options/locale/locale_en-US.ini

@@ -1654,6 +1654,10 @@ pulls.switch_comparison_type = Switch comparison type
 pulls.switch_head_and_base = Switch head and base
 pulls.filter_branch = Filter branch
 pulls.no_results = No results found.
+pulls.show_all_commits = Show all commits
+pulls.show_all_commits.description = Show all commits of this PR
+pulls.showing_only_single_commit = Showing only changes of commit %[1]s
+pulls.select_commits = Filter commit
 pulls.nothing_to_compare = These branches are equal. There is no need to create a pull request.
 pulls.nothing_to_compare_and_allow_empty_pr = These branches are equal. This PR will be empty.
 pulls.has_pull_request = `A pull request between these branches already exists: <a href="%[1]s">%[2]s#%[3]d</a>`

routers/web/repo/pull.go

@@ -683,6 +683,8 @@ func ViewPullCommits(ctx *context.Context) {
 
 // ViewPullFiles render pull request changed files list page
 func ViewPullFiles(ctx *context.Context) {
+	commitToShow := ctx.Params("sha")
+	willShowSingleCommit := len(commitToShow) > 0
 	ctx.Data["PageIsPullList"] = true
 	ctx.Data["PageIsPullFiles"] = true
 
@@ -719,8 +721,16 @@ func ViewPullFiles(ctx *context.Context) {
 	}
 
 	startCommitID = prInfo.MergeBase
-	endCommitID = headCommitID
 
+	if willShowSingleCommit {
+		endCommitID = commitToShow
+		ctx.Data["IsShowingAllCommits"] = false
+	} else {
+		endCommitID = headCommitID
+		ctx.Data["IsShowingAllCommits"] = true
+	}
+
+	ctx.Data["Commits"] = prInfo.Commits
 	ctx.Data["Username"] = ctx.Repo.Owner.Name
 	ctx.Data["Reponame"] = ctx.Repo.Repository.Name
 	ctx.Data["AfterCommitID"] = endCommitID
@@ -732,19 +742,35 @@ func ViewPullFiles(ctx *context.Context) {
 	if fileOnly && (len(files) == 2 || len(files) == 1) {
 		maxLines, maxFiles = -1, -1
 	}
-	diffOptions := &gitdiff.DiffOptions{
-		BeforeCommitID:     startCommitID,
-		AfterCommitID:      endCommitID,
-		SkipTo:             ctx.FormString("skip-to"),
-		MaxLines:           maxLines,
-		MaxLineCharacters:  setting.Git.MaxGitDiffLineCharacters,
-		MaxFiles:           maxFiles,
-		WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
+
+	var diffOptions *gitdiff.DiffOptions
+
+	// show only a single commit for this pr
+	if willShowSingleCommit {
+		diffOptions = &gitdiff.DiffOptions{
+			AfterCommitID:      endCommitID,
+			SkipTo:             ctx.FormString("skip-to"),
+			MaxLines:           maxLines,
+			MaxLineCharacters:  setting.Git.MaxGitDiffLineCharacters,
+			MaxFiles:           maxFiles,
+			WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
+		}
+	} else {
+		// show full PR diff
+		diffOptions = &gitdiff.DiffOptions{
+			BeforeCommitID:     startCommitID,
+			AfterCommitID:      endCommitID,
+			SkipTo:             ctx.FormString("skip-to"),
+			MaxLines:           maxLines,
+			MaxLineCharacters:  setting.Git.MaxGitDiffLineCharacters,
+			MaxFiles:           maxFiles,
+			WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
+		}
 	}
 
 	var methodWithError string
 	var diff *gitdiff.Diff
-	if !ctx.IsSigned {
+	if !ctx.IsSigned || willShowSingleCommit {
 		diff, err = gitdiff.GetDiff(gitRepo, diffOptions, files...)
 		methodWithError = "GetDiff"
 	} else {

routers/web/web.go

@@ -1286,6 +1286,7 @@ func registerRoutes(m *web.Route) {
 			m.Post("/cleanup", context.RepoMustNotBeArchived(), context.RepoRef(), repo.CleanUpPullRequest)
 			m.Group("/files", func() {
 				m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFiles)
+				m.Get("/{sha}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFiles)
 				m.Group("/reviews", func() {
 					m.Get("/new_comment", repo.RenderNewCodeCommentForm)
 					m.Post("/comments", web.Bind(forms.CodeCommentForm{}), repo.SetShowOutdatedComments, repo.CreateCodeComment)

templates/repo/diff/box.tmpl

@@ -43,11 +43,19 @@
 				{{end}}
 				{{template "repo/diff/whitespace_dropdown" .}}
 				{{template "repo/diff/options_dropdown" .}}
+				{{if .PageIsPullFiles}}
+					{{template "repo/diff/commits_dropdown" .}}
+				{{end}}
 				{{if and .PageIsPullFiles $.SignedUserID (not .IsArchived)}}
 					{{template "repo/diff/new_review" .}}
 				{{end}}
 			</div>
 		</div>
+		{{if and (not .IsShowingAllCommits) .PageIsPullFiles}}
+			<div class="ui info message">
+				<div class="text gt-whitespace-pre">{{.locale.Tr "repo.pulls.showing_only_single_commit" (ShortSha .AfterCommitID)}}</div>
+			</div>
+		{{end}}
 		<script id="diff-data-script" type="module">
 			const diffDataFiles = [{{range $i, $file := .Diff.Files}}{Name:"{{$file.Name}}",NameHash:"{{$file.NameHash}}",Type:{{$file.Type}},IsBin:{{$file.IsBin}},Addition:{{$file.Addition}},Deletion:{{$file.Deletion}},IsViewed:{{$file.IsViewed}}},{{end}}];
 			const diffData = {
@@ -88,7 +96,7 @@
 						{{$isCsv := (call $.IsCsvFile $file)}}
 						{{$showFileViewToggle := or $isImage (and (not $file.IsIncomplete) $isCsv)}}
 						{{$isExpandable := or (gt $file.Addition 0) (gt $file.Deletion 0) $file.IsBin}}
-						{{$isReviewFile := and $.IsSigned $.PageIsPullFiles (not $.IsArchived)}}
+						{{$isReviewFile := and $.IsSigned $.PageIsPullFiles (not $.IsArchived) $.IsShowingAllCommits}}
 						<div class="diff-file-box diff-box file-content {{TabSizeClass $.Editorconfig $file.Name}} gt-mt-3" id="diff-{{$file.NameHash}}" data-old-filename="{{$file.OldName}}" data-new-filename="{{$file.Name}}" {{if or ($file.ShouldBeHidden) (not $isExpandable)}}data-folded="true"{{end}}>
 							<h4 class="diff-file-header sticky-2nd-row ui top attached normal header gt-df gt-ac gt-sb gt-fw">
 								<div class="diff-file-name gt-df gt-ac gt-gap-2 gt-fw">
@@ -153,7 +161,7 @@
 									{{end}}
 								</div>
 							</h4>
-							<div class="diff-file-body ui attached unstackable table segment" {{if $file.IsViewed}}data-folded="true"{{end}}>
+							<div class="diff-file-body ui attached unstackable table segment" {{if and $file.IsViewed $.IsShowingAllCommits}}data-folded="true"{{end}}>
 								<div id="diff-source-{{$file.NameHash}}" class="file-body file-code unicode-escaped code-diff{{if $.IsSplitStyle}} code-diff-split{{else}} code-diff-unified{{end}}{{if $showFileViewToggle}} gt-hidden{{end}}">
 									{{if or $file.IsIncomplete $file.IsBin}}
 										<div class="diff-file-body binary" style="padding: 5px 10px;">

templates/repo/diff/commits_dropdown.tmpl

@@ -0,0 +1,32 @@
+<div class="ui dropdown tiny basic button" data-tooltip-content="{{.locale.Tr "repo.pulls.select_commits"}}">
+	{{svg "octicon-git-commit"}}
+	<div class="menu commit-selector-menu">
+		<a class="vertical item" href="{{$.Issue.Link}}/files?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}">
+			<span class="description gt-ellipsis">
+				{{.locale.Tr "repo.pulls.show_all_commits.description"}}
+			</span>
+			<span class="text gt-ellipsis">
+				{{.locale.Tr "repo.pulls.show_all_commits"}}
+			</span>
+		</a>
+
+		{{range .Commits}}
+			<div class="divider"></div>
+			<a class="vertical item" href="{{$.Issue.Link}}/files/{{.ID.String}}?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}">
+				<span class="description gt-ellipsis">
+					{{if .Committer}}
+						{{.Committer.Name}}
+						<span class="text right">{{TimeSince .Committer.When $.locale}}</span>
+					{{else}}
+						{{.Author.Name}}
+						<span class="text right">{{TimeSince .Author.When $.locale}}</span>
+					{{end}}
+				</span>
+				<div class="text">
+					<div class="ui compact gt-ellipsis commit-list-summary">{{.Summary}}</div>
+					<code class="ui right">{{ShortSha .ID.String}}</code>
+				</div>
+			</a>
+		{{end}}
+	</div>
+</div>

web_src/css/repo.css

@@ -3340,3 +3340,13 @@ tbody.commit-list {
   font-size: 18px;
   margin-left: 4px;
 }
+
+.commit-list-summary {
+  max-width: calc(100% - 10ch);
+  display: inline-block;
+}
+
+.commit-selector-menu {
+  max-height: 60vh;
+  overflow-y: auto;
+}