Use git diff-tree for DiffFileTree on diff pages (#33514)

Modify Diff View FileTree to show all files

## Changes

* removes Show Status button on diff
* uses `git diff-tree` to generate the file tree for the diff
* doesn't reload the diff tree each time we load more files in the
preview
* selecting and unloaded file will keep loading until that file is
loaded
* removes `DiffFileList.vue` and "Show Stats" in diff options

## Open Questions

* selecting and unloaded file will keep loading until that file is
loaded. Is this behaviour okay? It matches what github does.

### Demo

In this demo I set `git.MAX_GIT_DIFF_FILES=1` in my `app.ini` to
demonstrate a worst case example. In most cases the behaviour isn't
nearly as jarring as we load a bunch of files at a time.


https://github.com/user-attachments/assets/72f29663-d6fc-472d-94fa-7fb5950c2836

---------

Co-authored-by: silverwind <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>

Author: 3 people

options/locale/locale_en-US.ini

@@ -2594,7 +2594,6 @@ diff.commit = commit
 diff.git-notes = Notes
 diff.data_not_available = Diff Content Not Available
 diff.options_button = Diff Options
-diff.show_diff_stats = Show Stats
 diff.download_patch = Download Patch File
 diff.download_diff = Download Diff File
 diff.show_split_view = Split View

routers/web/repo/commit.go

@@ -344,18 +344,30 @@ func Diff(ctx *context.Context) {
 	ctx.Data["Reponame"] = repoName
 
 	var parentCommit *git.Commit
+	var parentCommitID string
 	if commit.ParentCount() > 0 {
 		parentCommit, err = gitRepo.GetCommit(parents[0])
 		if err != nil {
 			ctx.NotFound(err)
 			return
 		}
+		parentCommitID = parentCommit.ID.String()
 	}
 	setCompareContext(ctx, parentCommit, commit, userName, repoName)
 	ctx.Data["Title"] = commit.Summary() + " · " + base.ShortSha(commitID)
 	ctx.Data["Commit"] = commit
 	ctx.Data["Diff"] = diff
 
+	if !fileOnly {
+		diffTree, err := gitdiff.GetDiffTree(ctx, gitRepo, false, parentCommitID, commitID)
+		if err != nil {
+			ctx.ServerError("GetDiffTree", err)
+			return
+		}
+
+		ctx.PageData["DiffFiles"] = transformDiffTreeForUI(diffTree, nil)
+	}
+
 	statuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, commitID, db.ListOptionsAll)
 	if err != nil {
 		log.Error("GetLatestCommitStatus: %v", err)

routers/web/repo/compare.go

@@ -633,6 +633,16 @@ func PrepareCompareDiff(
 	ctx.Data["Diff"] = diff
 	ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0
 
+	if !fileOnly {
+		diffTree, err := gitdiff.GetDiffTree(ctx, ci.HeadGitRepo, false, beforeCommitID, headCommitID)
+		if err != nil {
+			ctx.ServerError("GetDiffTree", err)
+			return false
+		}
+
+		ctx.PageData["DiffFiles"] = transformDiffTreeForUI(diffTree, nil)
+	}
+
 	headCommit, err := ci.HeadGitRepo.GetCommit(headCommitID)
 	if err != nil {
 		ctx.ServerError("GetCommit", err)

routers/web/repo/pull.go

@@ -761,6 +761,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
 
 	var methodWithError string
 	var diff *gitdiff.Diff
+	shouldGetUserSpecificDiff := false
 
 	// if we're not logged in or only a single commit (or commit range) is shown we
 	// have to load only the diff and not get the viewed information
@@ -772,6 +773,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
 	} else {
 		diff, err = gitdiff.SyncAndGetUserSpecificDiff(ctx, ctx.Doer.ID, pull, gitRepo, diffOptions, files...)
 		methodWithError = "SyncAndGetUserSpecificDiff"
+		shouldGetUserSpecificDiff = true
 	}
 	if err != nil {
 		ctx.ServerError(methodWithError, err)
@@ -816,6 +818,27 @@ 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, pull.MergeBase, headCommitID)
+		if err != nil {
+			ctx.ServerError("GetDiffTree", err)
+			return
+		}
+
+		filesViewedState := make(map[string]pull_model.ViewedState)
+		if shouldGetUserSpecificDiff {
+			// This sort of sucks because we already fetch this when getting the diff
+			review, err := pull_model.GetNewestReviewState(ctx, ctx.Doer.ID, issue.ID)
+			if err == nil && review != nil && review.UpdatedFiles != nil {
+				// If there wasn't an error and we have a review with updated files, use that
+				filesViewedState = review.UpdatedFiles
+			}
+		}
+
+		ctx.PageData["DiffFiles"] = transformDiffTreeForUI(diffTree, filesViewedState)
+	}
+
 	ctx.Data["Diff"] = diff
 	ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0
 

routers/web/repo/treelist.go

@@ -6,9 +6,11 @@ package repo
 import (
 	"net/http"
 
+	pull_model "code.gitea.io/gitea/models/pull"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/services/context"
+	"code.gitea.io/gitea/services/gitdiff"
 
 	"github.com/go-enry/go-enry/v2"
 )
@@ -52,3 +54,33 @@ func isExcludedEntry(entry *git.TreeEntry) bool {
 
 	return false
 }
+
+type FileDiffFile struct {
+	Name        string
+	NameHash    string
+	IsSubmodule bool
+	IsViewed    bool
+	Status      string
+}
+
+// transformDiffTreeForUI transforms a DiffTree into a slice of FileDiffFile for UI rendering
+// it also takes a map of file names to their viewed state, which is used to mark files as viewed
+func transformDiffTreeForUI(diffTree *gitdiff.DiffTree, filesViewedState map[string]pull_model.ViewedState) []FileDiffFile {
+	files := make([]FileDiffFile, 0, len(diffTree.Files))
+
+	for _, file := range diffTree.Files {
+		nameHash := git.HashFilePathForWebUI(file.HeadPath)
+		isSubmodule := file.HeadMode == git.EntryModeCommit
+		isViewed := filesViewedState[file.HeadPath] == pull_model.Viewed
+
+		files = append(files, FileDiffFile{
+			Name:        file.HeadPath,
+			NameHash:    nameHash,
+			IsSubmodule: isSubmodule,
+			IsViewed:    isViewed,
+			Status:      file.Status,
+		})
+	}
+
+	return files
+}

templates/repo/diff/box.tmpl

@@ -57,32 +57,6 @@
 				<div>{{ctx.Locale.Tr "repo.pulls.showing_specified_commit_range" (ShortSha .BeforeCommitID) (ShortSha .AfterCommitID)}} - <a href="{{$.Issue.Link}}/files?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}">{{ctx.Locale.Tr "repo.pulls.show_all_commits"}}</a></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}},IsSubmodule:{{$file.IsSubmodule}},Addition:{{$file.Addition}},Deletion:{{$file.Deletion}},IsViewed:{{$file.IsViewed}}},{{end}}];
-			const diffData = {
-				isIncomplete: {{.Diff.IsIncomplete}},
-				tooManyFilesMessage: "{{ctx.Locale.Tr "repo.diff.too_many_files"}}",
-				binaryFileMessage: "{{ctx.Locale.Tr "repo.diff.bin"}}",
-				showMoreMessage: "{{ctx.Locale.Tr "repo.diff.show_more"}}",
-				statisticsMessage: "{{ctx.Locale.Tr "repo.diff.stats_desc_file"}}",
-				linkLoadMore: "?skip-to={{.Diff.End}}&file-only=true",
-			};
-
-			// for first time loading, the diffFileInfo is a plain object
-			// after the Vue component is mounted, the diffFileInfo is a reactive object
-			// keep in mind that this script block would be executed many times when loading more files, by "loadMoreFiles"
-			let diffFileInfo = window.config.pageData.diffFileInfo || {
-				files:[],
-				fileTreeIsVisible: false,
-				fileListIsVisible: false,
-				isLoadingNewData: false,
-				selectedItem: '',
-			};
-			diffFileInfo = Object.assign(diffFileInfo, diffData);
-			diffFileInfo.files.push(...diffDataFiles);
-			window.config.pageData.diffFileInfo = diffFileInfo;
-		</script>
-		<div id="diff-file-list"></div>
 	{{end}}
 	<div id="diff-container">
 		{{if $showFileTree}}

templates/repo/diff/options_dropdown.tmpl

@@ -1,7 +1,6 @@
 <div class="ui dropdown tiny basic button" data-tooltip-content="{{ctx.Locale.Tr "repo.diff.options_button"}}">
 	{{svg "octicon-kebab-horizontal"}}
 	<div class="menu">
-		<a class="item" id="show-file-list-btn">{{ctx.Locale.Tr "repo.diff.show_diff_stats"}}</a>
 		{{if .Issue.Index}}
 			<a class="item" href="{{$.RepoLink}}/pulls/{{.Issue.Index}}.patch" download="{{.Issue.Index}}.patch">{{ctx.Locale.Tr "repo.diff.download_patch"}}</a>
 			<a class="item" href="{{$.RepoLink}}/pulls/{{.Issue.Index}}.diff" download="{{.Issue.Index}}.diff">{{ctx.Locale.Tr "repo.diff.download_diff"}}</a>

web_src/js/components/DiffFileList.vue

@@ -1,75 +1,18 @@
 <script lang="ts" setup>
-import DiffFileTreeItem, {type Item} from './DiffFileTreeItem.vue';
-import {loadMoreFiles} from '../features/repo-diff.ts';
+import DiffFileTreeItem from './DiffFileTreeItem.vue';
 import {toggleElem} from '../utils/dom.ts';
 import {diffTreeStore} from '../modules/stores.ts';
 import {setFileFolding} from '../features/file-fold.ts';
 import {computed, onMounted, onUnmounted} from 'vue';
+import {pathListToTree, mergeChildIfOnlyOneDir} from '../utils/filetree.ts';
 
 const LOCAL_STORAGE_KEY = 'diff_file_tree_visible';
 
 const store = diffTreeStore();
 
 const fileTree = computed(() => {
-  const result: Array<Item> = [];
-  for (const file of store.files) {
-    // Split file into directories
-    const splits = file.Name.split('/');
-    let index = 0;
-    let parent = null;
-    let isFile = false;
-    for (const split of splits) {
-      index += 1;
-      // reached the end
-      if (index === splits.length) {
-        isFile = true;
-      }
-      let newParent: Item = {
-        name: split,
-        children: [],
-        isFile,
-      };
-
-      if (isFile === true) {
-        newParent.file = file;
-      }
-
-      if (parent) {
-        // check if the folder already exists
-        const existingFolder = parent.children.find(
-          (x) => x.name === split,
-        );
-        if (existingFolder) {
-          newParent = existingFolder;
-        } else {
-          parent.children.push(newParent);
-        }
-      } else {
-        const existingFolder = result.find((x) => x.name === split);
-        if (existingFolder) {
-          newParent = existingFolder;
-        } else {
-          result.push(newParent);
-        }
-      }
-      parent = newParent;
-    }
-  }
-  const mergeChildIfOnlyOneDir = (entries: Array<Record<string, any>>) => {
-    for (const entry of entries) {
-      if (entry.children) {
-        mergeChildIfOnlyOneDir(entry.children);
-      }
-      if (entry.children.length === 1 && entry.children[0].isFile === false) {
-        // Merge it to the parent
-        entry.name = `${entry.name}/${entry.children[0].name}`;
-        entry.children = entry.children[0].children;
-      }
-    }
-  };
-  // Merge folders with just a folder as children in order to
-  // reduce the depth of our tree.
-  mergeChildIfOnlyOneDir(result);
+  const result = pathListToTree(store.files);
+  mergeChildIfOnlyOneDir(result); // mutation
   return result;
 });
 
@@ -121,19 +64,12 @@ function updateState(visible: boolean) {
   toggleElem(toShow, !visible);
   toggleElem(toHide, visible);
 }
-
-function loadMoreData() {
-  loadMoreFiles(store.linkLoadMore);
-}
 </script>
 
 <template>
   <div v-if="store.fileTreeIsVisible" class="diff-file-tree-items">
     <!-- only render the tree if we're visible. in many cases this is something that doesn't change very often -->
     <DiffFileTreeItem v-for="item in fileTree" :key="item.name" :item="item"/>
-    <div v-if="store.isIncomplete" class="tw-pt-1">
-      <a :class="['ui', 'basic', 'tiny', 'button', store.isLoadingNewData ? 'disabled' : '']" @click.stop="loadMoreData">{{ store.showMoreMessage }}</a>
-    </div>
   </div>
 </template>