From 4799444420e4794953a8bf5257a29994f882def7 Mon Sep 17 00:00:00 2001 From: Alexander McRae Date: Wed, 5 Feb 2025 19:36:07 -0800 Subject: [PATCH] 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 --- routers/web/repo/pull.go | 8 +++ routers/web/repo/treelist.go | 39 +++++++++++ services/gitdiff/git_diff_tree.go | 7 ++ templates/repo/diff/box.tmpl | 3 +- templates/repo/diff/options_dropdown.tmpl | 1 - web_src/js/components/DiffFileList.vue | 60 ----------------- web_src/js/components/DiffFileTree.vue | 62 +----------------- web_src/js/components/DiffFileTreeItem.vue | 33 +++------- web_src/js/components/file_tree.ts | 76 ++++++++++++++++++++++ web_src/js/features/repo-diff-filetree.ts | 8 --- web_src/js/features/repo-diff.ts | 3 +- 11 files changed, 146 insertions(+), 154 deletions(-) delete mode 100644 web_src/js/components/DiffFileList.vue create mode 100644 web_src/js/components/file_tree.ts diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index e6fb492d6e370..9963e1df18e45 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -812,6 +812,14 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } } + // 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 + } + + ctx.Data["DiffFiles"] = transformDiffTreeForUI(diffTree, nil) ctx.Data["Diff"] = diff ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0 diff --git a/routers/web/repo/treelist.go b/routers/web/repo/treelist.go index d11af4669f90c..04bf3ba298806 100644 --- a/routers/web/repo/treelist.go +++ b/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,40 @@ func isExcludedEntry(entry *git.TreeEntry) bool { return false } + +type FileDiffFile struct { + Name string + NameHash string + IsSubmodule bool + IsBinary 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 + isBinary := file.HeadMode == git.EntryModeExec + + isViewed := false + if fileViewedState, ok := filesViewedState[file.Path()]; ok { + isViewed = (fileViewedState == pull_model.Viewed) + } + + files = append(files, FileDiffFile{ + Name: file.HeadPath, + NameHash: nameHash, + IsSubmodule: isSubmodule, + IsBinary: isBinary, + IsViewed: isViewed, + Status: file.Status, + }) + } + + return files +} diff --git a/services/gitdiff/git_diff_tree.go b/services/gitdiff/git_diff_tree.go index 8039de145d2db..16b40bf508d20 100644 --- a/services/gitdiff/git_diff_tree.go +++ b/services/gitdiff/git_diff_tree.go @@ -34,6 +34,13 @@ type DiffTreeRecord struct { BaseBlobID string } +func (d *DiffTreeRecord) Path() string { + if d.HeadPath != "" { + return d.HeadPath + } + return d.BasePath +} + // GetDiffTree returns the list of path of the files that have changed between the two commits. // If useMergeBase is true, the diff will be calculated using the merge base of the two commits. // This is the same behavior as using a three-dot diff in git diff. diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index a3b64b8a11f79..1315e45cbe3de 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -58,7 +58,8 @@ {{end}} - - diff --git a/web_src/js/components/DiffFileTree.vue b/web_src/js/components/DiffFileTree.vue index d00d03565f217..661efbb56cdee 100644 --- a/web_src/js/components/DiffFileTree.vue +++ b/web_src/js/components/DiffFileTree.vue @@ -5,71 +5,15 @@ 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 './file_tree.ts'; const LOCAL_STORAGE_KEY = 'diff_file_tree_visible'; const store = diffTreeStore(); const fileTree = computed(() => { - const result: Array = []; - 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>) => { - 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); + let result = pathListToTree(store.files); + mergeChildIfOnlyOneDir(result); // mutation return result; }); diff --git a/web_src/js/components/DiffFileTreeItem.vue b/web_src/js/components/DiffFileTreeItem.vue index d3be10e3e9ca5..b89b367ad8769 100644 --- a/web_src/js/components/DiffFileTreeItem.vue +++ b/web_src/js/components/DiffFileTreeItem.vue @@ -2,21 +2,8 @@ import {SvgIcon, type SvgName} from '../svg.ts'; import {diffTreeStore} from '../modules/stores.ts'; import {ref} from 'vue'; +import { type Item, type File, type FileStatus } from './file_tree.ts'; -type File = { - Name: string; - NameHash: string; - Type: number; - IsViewed: boolean; - IsSubmodule: boolean; -} - -export type Item = { - name: string; - isFile: boolean; - file?: File; - children?: Item[]; -}; defineProps<{ item: Item, @@ -25,15 +12,15 @@ defineProps<{ const store = diffTreeStore(); const collapsed = ref(false); -function getIconForDiffType(pType: number) { - const diffTypes: Record}> = { - '1': {name: 'octicon-diff-added', classes: ['text', 'green']}, - '2': {name: 'octicon-diff-modified', classes: ['text', 'yellow']}, - '3': {name: 'octicon-diff-removed', classes: ['text', 'red']}, - '4': {name: 'octicon-diff-renamed', classes: ['text', 'teal']}, - '5': {name: 'octicon-diff-renamed', classes: ['text', 'green']}, // there is no octicon for copied, so renamed should be ok +function getIconForDiffType(pType: FileStatus) { + const diffTypes: Record}> = { + 'added': {name: 'octicon-diff-added', classes: ['text', 'green']}, + 'modified': {name: 'octicon-diff-modified', classes: ['text', 'yellow']}, + 'deleted': {name: 'octicon-diff-removed', classes: ['text', 'red']}, + 'renamed': {name: 'octicon-diff-renamed', classes: ['text', 'teal']}, + 'typechange': {name: 'octicon-diff-modified', classes: ['text', 'green']}, // there is no octicon for copied, so renamed should be ok }; - return diffTypes[String(pType)]; + return diffTypes[pType]; } function fileIcon(file: File) { @@ -54,7 +41,7 @@ function fileIcon(file: File) { {{ item.name }} - +
diff --git a/web_src/js/components/file_tree.ts b/web_src/js/components/file_tree.ts new file mode 100644 index 0000000000000..dd8ea52a4cf53 --- /dev/null +++ b/web_src/js/components/file_tree.ts @@ -0,0 +1,76 @@ +export type File = { + Name: string; + NameHash: string; + Status: FileStatus; + IsViewed: boolean; + IsSubmodule: boolean; +} +export type FileStatus = "added" | "modified" | "deleted" | "renamed" | "typechange"; + +export type Item = { + name: string; + path: string; + isFile: boolean; + file?: File; + children?: Item[]; +}; + +export function pathListToTree(fileEntries: File[]): Item[] { + const pathToItem = new Map(); + + // init root node + const root: Item = { name: '', path: '', isFile: false, children: [] }; + pathToItem.set('', root); + + for (const fileEntry of fileEntries) { + const [parentPath, fileName] = splitPathLast(fileEntry.Name); + + let parentItem = pathToItem.get(parentPath); + if (!parentItem) { + parentItem = constructParents(pathToItem, parentPath); + } + + const fileItem: Item = { name: fileName, path: fileEntry.Name, isFile: true, file: fileEntry }; + + parentItem.children.push(fileItem); + } + + return root.children; +} + + +function constructParents(pathToItem: Map, dirPath: string): Item { + const [dirParentPath, dirName] = splitPathLast(dirPath); + + let parentItem = pathToItem.get(dirParentPath); + if (!parentItem) { + // if the parent node does not exist, create it + parentItem = constructParents(pathToItem, dirParentPath); + } + + const dirItem: Item = { name: dirName, path: dirPath, isFile: false, children: [] }; + parentItem.children.push(dirItem); + pathToItem.set(dirPath, dirItem); + + return dirItem; +} + +function splitPathLast(path: string): [string, string] { + const lastSlash = path.lastIndexOf('/'); + return [path.substring(0, lastSlash), path.substring(lastSlash + 1)]; +} + +export function mergeChildIfOnlyOneDir(nodes: Item[]): void { + for (const node of nodes) { + if (node.children) { + mergeChildIfOnlyOneDir(node.children); + } + + if (node.children?.length === 1 && node.children[0].children) { + const child = node.children[0]; + node.name = `${node.name}/${child.name}`; + node.path = child.path; + node.children = child.children; + } + } +} diff --git a/web_src/js/features/repo-diff-filetree.ts b/web_src/js/features/repo-diff-filetree.ts index bc275a90f6ab3..6d4bba27a86a5 100644 --- a/web_src/js/features/repo-diff-filetree.ts +++ b/web_src/js/features/repo-diff-filetree.ts @@ -1,6 +1,5 @@ import {createApp} from 'vue'; import DiffFileTree from '../components/DiffFileTree.vue'; -import DiffFileList from '../components/DiffFileList.vue'; export function initDiffFileTree() { const el = document.querySelector('#diff-file-tree'); @@ -10,10 +9,3 @@ export function initDiffFileTree() { fileTreeView.mount(el); } -export function initDiffFileList() { - const fileListElement = document.querySelector('#diff-file-list'); - if (!fileListElement) return; - - const fileListView = createApp(DiffFileList); - fileListView.mount(fileListElement); -} diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 0dad4da86226f..50e68717297f7 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -1,7 +1,7 @@ import $ from 'jquery'; import {initCompReactionSelector} from './comp/ReactionSelector.ts'; import {initRepoIssueContentHistory} from './repo-issue-content.ts'; -import {initDiffFileTree, initDiffFileList} from './repo-diff-filetree.ts'; +import {initDiffFileTree} from './repo-diff-filetree.ts'; import {initDiffCommitSelect} from './repo-diff-commitselect.ts'; import {validateTextareaNonEmpty} from './comp/ComboMarkdownEditor.ts'; import {initViewedCheckboxListenerFor, countAndUpdateViewedFiles, initExpandAndCollapseFilesButton} from './pull-view-file.ts'; @@ -238,7 +238,6 @@ export function initRepoDiffView() { initRepoDiffConversationForm(); if (!$('#diff-file-list').length) return; initDiffFileTree(); - initDiffFileList(); initDiffCommitSelect(); initRepoDiffShowMore(); initDiffHeaderPopup();