diff options
author | Alexander McRae <alex.mcrae@snailscale.io> | 2025-02-27 16:58:25 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2025-02-28 00:58:25 +0000 |
commit | aba96f65cd687868c6eff5a079cfc7b25740642d (patch) | |
tree | 00920810b133115323d1a08eb59f5e00b3ff6e02 | |
parent | 7a8eed13b9113850cbb7845b44c193594c7ad6d1 (diff) | |
download | gitea-main.tar.gz gitea-main.zip |
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 <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
-rw-r--r-- | options/locale/locale_en-US.ini | 1 | ||||
-rw-r--r-- | routers/web/repo/commit.go | 12 | ||||
-rw-r--r-- | routers/web/repo/compare.go | 10 | ||||
-rw-r--r-- | routers/web/repo/pull.go | 23 | ||||
-rw-r--r-- | routers/web/repo/treelist.go | 32 | ||||
-rw-r--r-- | templates/repo/diff/box.tmpl | 26 | ||||
-rw-r--r-- | templates/repo/diff/options_dropdown.tmpl | 1 | ||||
-rw-r--r-- | web_src/js/components/DiffFileList.vue | 60 | ||||
-rw-r--r-- | web_src/js/components/DiffFileTree.vue | 72 | ||||
-rw-r--r-- | web_src/js/components/DiffFileTreeItem.vue | 67 | ||||
-rw-r--r-- | web_src/js/features/repo-diff-filetree.ts | 9 | ||||
-rw-r--r-- | web_src/js/features/repo-diff.ts | 69 | ||||
-rw-r--r-- | web_src/js/modules/stores.ts | 9 | ||||
-rw-r--r-- | web_src/js/utils.test.ts | 8 | ||||
-rw-r--r-- | web_src/js/utils.ts | 6 | ||||
-rw-r--r-- | web_src/js/utils/filetree.test.ts | 86 | ||||
-rw-r--r-- | web_src/js/utils/filetree.ts | 85 |
17 files changed, 352 insertions, 224 deletions
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 5aa6d19dae..1da2fb61ad 100644 --- a/options/locale/locale_en-US.ini +++ b/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 diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 9c12ef9297..2728eda8b6 100644 --- a/routers/web/repo/commit.go +++ b/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) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 71bce759a9..5165636a52 100644 --- a/routers/web/repo/compare.go +++ b/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) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 223f8d017e..0769f456ec 100644 --- a/routers/web/repo/pull.go +++ b/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 diff --git a/routers/web/repo/treelist.go b/routers/web/repo/treelist.go index d11af4669f..9ce9f8424d 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,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 +} diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index a3b64b8a11..8de54b932a 100644 --- a/templates/repo/diff/box.tmpl +++ b/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}} diff --git a/templates/repo/diff/options_dropdown.tmpl b/templates/repo/diff/options_dropdown.tmpl index 09b7b80e41..8d08e7ad46 100644 --- a/templates/repo/diff/options_dropdown.tmpl +++ b/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> diff --git a/web_src/js/components/DiffFileList.vue b/web_src/js/components/DiffFileList.vue deleted file mode 100644 index 6570c92781..0000000000 --- a/web_src/js/components/DiffFileList.vue +++ /dev/null @@ -1,60 +0,0 @@ -<script lang="ts" setup> -import {onMounted, onUnmounted} from 'vue'; -import {loadMoreFiles} from '../features/repo-diff.ts'; -import {diffTreeStore} from '../modules/stores.ts'; - -const store = diffTreeStore(); - -onMounted(() => { - document.querySelector('#show-file-list-btn').addEventListener('click', toggleFileList); -}); - -onUnmounted(() => { - document.querySelector('#show-file-list-btn').removeEventListener('click', toggleFileList); -}); - -function toggleFileList() { - store.fileListIsVisible = !store.fileListIsVisible; -} - -function diffTypeToString(pType: number) { - const diffTypes: Record<string, string> = { - '1': 'add', - '2': 'modify', - '3': 'del', - '4': 'rename', - '5': 'copy', - }; - return diffTypes[String(pType)]; -} - -function diffStatsWidth(adds: number, dels: number) { - return `${adds / (adds + dels) * 100}%`; -} - -function loadMoreData() { - loadMoreFiles(store.linkLoadMore); -} -</script> - -<template> - <ol class="diff-stats tw-m-0" ref="root" v-if="store.fileListIsVisible"> - <li v-for="file in store.files" :key="file.NameHash"> - <div class="tw-font-semibold tw-flex tw-items-center pull-right"> - <span v-if="file.IsBin" class="tw-ml-0.5 tw-mr-2">{{ store.binaryFileMessage }}</span> - {{ file.IsBin ? '' : file.Addition + file.Deletion }} - <span v-if="!file.IsBin" class="diff-stats-bar tw-mx-2" :data-tooltip-content="store.statisticsMessage.replace('%d', (file.Addition + file.Deletion)).replace('%d', file.Addition).replace('%d', file.Deletion)"> - <div class="diff-stats-add-bar" :style="{ 'width': diffStatsWidth(file.Addition, file.Deletion) }"/> - </span> - </div> - <!-- todo finish all file status, now modify, add, delete and rename --> - <span :class="['status', diffTypeToString(file.Type)]" :data-tooltip-content="diffTypeToString(file.Type)"> </span> - <a class="file tw-font-mono" :href="'#diff-' + file.NameHash">{{ file.Name }}</a> - </li> - <li v-if="store.isIncomplete" class="tw-pt-1"> - <span class="file tw-flex tw-items-center tw-justify-between">{{ store.tooManyFilesMessage }} - <a :class="['ui', 'basic', 'tiny', 'button', store.isLoadingNewData ? 'disabled' : '']" @click.stop="loadMoreData">{{ store.showMoreMessage }}</a> - </span> - </li> - </ol> -</template> diff --git a/web_src/js/components/DiffFileTree.vue b/web_src/js/components/DiffFileTree.vue index d00d03565f..381a1c3ca4 100644 --- a/web_src/js/components/DiffFileTree.vue +++ b/web_src/js/components/DiffFileTree.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> diff --git a/web_src/js/components/DiffFileTreeItem.vue b/web_src/js/components/DiffFileTreeItem.vue index d3be10e3e9..5ee0e5bcaa 100644 --- a/web_src/js/components/DiffFileTreeItem.vue +++ b/web_src/js/components/DiffFileTreeItem.vue @@ -2,21 +2,7 @@ import {SvgIcon, type SvgName} from '../svg.ts'; import {diffTreeStore} from '../modules/stores.ts'; import {ref} from 'vue'; - -type File = { - Name: string; - NameHash: string; - Type: number; - IsViewed: boolean; - IsSubmodule: boolean; -} - -export type Item = { - name: string; - isFile: boolean; - file?: File; - children?: Item[]; -}; +import type {Item, File, FileStatus} from '../utils/filetree.ts'; defineProps<{ item: Item, @@ -25,15 +11,16 @@ defineProps<{ const store = diffTreeStore(); const collapsed = ref(false); -function getIconForDiffType(pType: number) { - const diffTypes: Record<string, {name: SvgName, classes: Array<string>}> = { - '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 getIconForDiffStatus(pType: FileStatus) { + const diffTypes: Record<FileStatus, { name: SvgName, classes: Array<string> }> = { + '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']}, + 'copied': {name: 'octicon-diff-renamed', classes: ['text', 'green']}, + '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) { @@ -48,27 +35,37 @@ function fileIcon(file: File) { <!--title instead of tooltip above as the tooltip needs too much work with the current methods, i.e. not being loaded or staying open for "too long"--> <a v-if="item.isFile" class="item-file" - :class="{'selected': store.selectedItem === '#diff-' + item.file.NameHash, 'viewed': item.file.IsViewed}" + :class="{ 'selected': store.selectedItem === '#diff-' + item.file.NameHash, 'viewed': item.file.IsViewed }" :title="item.name" :href="'#diff-' + item.file.NameHash" > <!-- file --> <SvgIcon :name="fileIcon(item.file)"/> <span class="gt-ellipsis tw-flex-1">{{ item.name }}</span> - <SvgIcon :name="getIconForDiffType(item.file.Type).name" :class="getIconForDiffType(item.file.Type).classes"/> + <SvgIcon + :name="getIconForDiffStatus(item.file.Status).name" + :class="getIconForDiffStatus(item.file.Status).classes" + /> </a> - <div v-else class="item-directory" :title="item.name" @click.stop="collapsed = !collapsed"> - <!-- directory --> - <SvgIcon :name="collapsed ? 'octicon-chevron-right' : 'octicon-chevron-down'"/> - <SvgIcon class="text primary" :name="collapsed ? 'octicon-file-directory-fill' : 'octicon-file-directory-open-fill'"/> - <span class="gt-ellipsis">{{ item.name }}</span> - </div> - <div v-if="item.children?.length" v-show="!collapsed" class="sub-items"> - <DiffFileTreeItem v-for="childItem in item.children" :key="childItem.name" :item="childItem"/> - </div> + <template v-else-if="item.isFile === false"> + <div class="item-directory" :title="item.name" @click.stop="collapsed = !collapsed"> + <!-- directory --> + <SvgIcon :name="collapsed ? 'octicon-chevron-right' : 'octicon-chevron-down'"/> + <SvgIcon + class="text primary" + :name="collapsed ? 'octicon-file-directory-fill' : 'octicon-file-directory-open-fill'" + /> + <span class="gt-ellipsis">{{ item.name }}</span> + </div> + + <div v-show="!collapsed" class="sub-items"> + <DiffFileTreeItem v-for="childItem in item.children" :key="childItem.name" :item="childItem"/> + </div> + </template> </template> <style scoped> -a, a:hover { +a, +a:hover { text-decoration: none; color: var(--color-text); } diff --git a/web_src/js/features/repo-diff-filetree.ts b/web_src/js/features/repo-diff-filetree.ts index bc275a90f6..cc4576a846 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'); @@ -9,11 +8,3 @@ export function initDiffFileTree() { const fileTreeView = createApp(DiffFileTree); 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 0dad4da862..85b84ee7ec 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'; @@ -21,7 +21,7 @@ import {fomanticQuery} from '../modules/fomantic/base.ts'; import {createTippy} from '../modules/tippy.ts'; import {invertFileFolding} from './file-fold.ts'; -const {pageData, i18n} = window.config; +const {i18n} = window.config; function initRepoDiffFileViewToggle() { $('.file-view-toggle').on('click', function () { @@ -161,6 +161,7 @@ function initDiffHeaderPopup() { // Will be called when the show more (files) button has been pressed function onShowMoreFiles() { + // FIXME: here the init calls are incomplete: at least it misses dropdown & initCompReactionSelector initRepoIssueContentHistory(); initViewedCheckboxListenerFor(); countAndUpdateViewedFiles(); @@ -168,41 +169,35 @@ function onShowMoreFiles() { initDiffHeaderPopup(); } -export async function loadMoreFiles(url: string) { - const target = document.querySelector('a#diff-show-more-files'); - if (target?.classList.contains('disabled') || pageData.diffFileInfo.isLoadingNewData) { - return; +async function loadMoreFiles(btn: Element): Promise<boolean> { + if (btn.classList.contains('disabled')) { + return false; } - pageData.diffFileInfo.isLoadingNewData = true; - target?.classList.add('disabled'); - + btn.classList.add('disabled'); + const url = btn.getAttribute('data-href'); try { const response = await GET(url); const resp = await response.text(); const $resp = $(resp); // the response is a full HTML page, we need to extract the relevant contents: - // 1. append the newly loaded file list items to the existing list + // * append the newly loaded file list items to the existing list $('#diff-incomplete').replaceWith($resp.find('#diff-file-boxes').children()); - // 2. re-execute the script to append the newly loaded items to the JS variables to refresh the DiffFileTree - $('body').append($resp.find('script#diff-data-script')); - onShowMoreFiles(); + return true; } catch (error) { console.error('Error:', error); showErrorToast('An error occurred while loading more files.'); } finally { - target?.classList.remove('disabled'); - pageData.diffFileInfo.isLoadingNewData = false; + btn.classList.remove('disabled'); } + return false; } function initRepoDiffShowMore() { - $(document).on('click', 'a#diff-show-more-files', (e) => { + addDelegatedEventListener(document, 'click', 'a#diff-show-more-files', (el, e) => { e.preventDefault(); - - const linkLoadMore = e.target.getAttribute('data-href'); - loadMoreFiles(linkLoadMore); + loadMoreFiles(el); }); $(document).on('click', 'a.diff-load-button', async (e) => { @@ -234,17 +229,49 @@ function initRepoDiffShowMore() { }); } +async function loadUntilFound() { + const hashTargetSelector = window.location.hash; + if (!hashTargetSelector.startsWith('#diff-') && !hashTargetSelector.startsWith('#issuecomment-')) { + return; + } + + while (true) { + // use getElementById to avoid querySelector throws an error when the hash is invalid + // eslint-disable-next-line unicorn/prefer-query-selector + const targetElement = document.getElementById(hashTargetSelector.substring(1)); + if (targetElement) { + targetElement.scrollIntoView(); + return; + } + + // the button will be refreshed after each "load more", so query it every time + const showMoreButton = document.querySelector('#diff-show-more-files'); + if (!showMoreButton) { + return; // nothing more to load + } + + // Load more files, await ensures we don't block progress + const ok = await loadMoreFiles(showMoreButton); + if (!ok) return; // failed to load more files + } +} + +function initRepoDiffHashChangeListener() { + window.addEventListener('hashchange', loadUntilFound); + loadUntilFound(); +} + export function initRepoDiffView() { initRepoDiffConversationForm(); - if (!$('#diff-file-list').length) return; + if (!$('#diff-file-boxes').length) return; initDiffFileTree(); - initDiffFileList(); initDiffCommitSelect(); initRepoDiffShowMore(); initDiffHeaderPopup(); initRepoDiffFileViewToggle(); initViewedCheckboxListenerFor(); initExpandAndCollapseFilesButton(); + initRepoDiffHashChangeListener(); addDelegatedEventListener(document, 'click', '.fold-file', (el) => { invertFileFolding(el.closest('.file-content'), el); diff --git a/web_src/js/modules/stores.ts b/web_src/js/modules/stores.ts index 942a7bc508..65da1e044a 100644 --- a/web_src/js/modules/stores.ts +++ b/web_src/js/modules/stores.ts @@ -1,11 +1,16 @@ import {reactive} from 'vue'; import type {Reactive} from 'vue'; +const {pageData} = window.config; + let diffTreeStoreReactive: Reactive<Record<string, any>>; export function diffTreeStore() { if (!diffTreeStoreReactive) { - diffTreeStoreReactive = reactive(window.config.pageData.diffFileInfo); - window.config.pageData.diffFileInfo = diffTreeStoreReactive; + diffTreeStoreReactive = reactive({ + files: pageData.DiffFiles, + fileTreeIsVisible: false, + selectedItem: '', + }); } return diffTreeStoreReactive; } diff --git a/web_src/js/utils.test.ts b/web_src/js/utils.test.ts index ccdbc2dbd7..f1025471a4 100644 --- a/web_src/js/utils.test.ts +++ b/web_src/js/utils.test.ts @@ -1,9 +1,15 @@ import { - basename, extname, isObject, stripTags, parseIssueHref, + dirname, basename, extname, isObject, stripTags, parseIssueHref, parseUrl, translateMonth, translateDay, blobToDataURI, toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64, isImageFile, isVideoFile, parseRepoOwnerPathInfo, } from './utils.ts'; +test('dirname', () => { + expect(dirname('/path/to/file.js')).toEqual('/path/to'); + expect(dirname('/path/to')).toEqual('/path'); + expect(dirname('file.js')).toEqual(''); +}); + test('basename', () => { expect(basename('/path/to/file.js')).toEqual('file.js'); expect(basename('/path/to/file')).toEqual('file'); diff --git a/web_src/js/utils.ts b/web_src/js/utils.ts index 54f59a2c03..b825a9339d 100644 --- a/web_src/js/utils.ts +++ b/web_src/js/utils.ts @@ -1,6 +1,12 @@ import {decode, encode} from 'uint8-to-base64'; import type {IssuePageInfo, IssuePathInfo, RepoOwnerPathInfo} from './types.ts'; +// transform /path/to/file.ext to /path/to +export function dirname(path: string): string { + const lastSlashIndex = path.lastIndexOf('/'); + return lastSlashIndex < 0 ? '' : path.substring(0, lastSlashIndex); +} + // transform /path/to/file.ext to file.ext export function basename(path: string): string { const lastSlashIndex = path.lastIndexOf('/'); diff --git a/web_src/js/utils/filetree.test.ts b/web_src/js/utils/filetree.test.ts new file mode 100644 index 0000000000..f561cb75f0 --- /dev/null +++ b/web_src/js/utils/filetree.test.ts @@ -0,0 +1,86 @@ +import {mergeChildIfOnlyOneDir, pathListToTree, type File} from './filetree.ts'; + +const emptyList: File[] = []; +const singleFile = [{Name: 'file1'}] as File[]; +const singleDir = [{Name: 'dir1/file1'}] as File[]; +const nestedDir = [{Name: 'dir1/dir2/file1'}] as File[]; +const multiplePathsDisjoint = [{Name: 'dir1/dir2/file1'}, {Name: 'dir3/file2'}] as File[]; +const multiplePathsShared = [{Name: 'dir1/dir2/dir3/file1'}, {Name: 'dir1/file2'}] as File[]; + +test('pathListToTree', () => { + expect(pathListToTree(emptyList)).toEqual([]); + expect(pathListToTree(singleFile)).toEqual([ + {isFile: true, name: 'file1', path: 'file1', file: {Name: 'file1'}}, + ]); + expect(pathListToTree(singleDir)).toEqual([ + {isFile: false, name: 'dir1', path: 'dir1', children: [ + {isFile: true, name: 'file1', path: 'dir1/file1', file: {Name: 'dir1/file1'}}, + ]}, + ]); + expect(pathListToTree(nestedDir)).toEqual([ + {isFile: false, name: 'dir1', path: 'dir1', children: [ + {isFile: false, name: 'dir2', path: 'dir1/dir2', children: [ + {isFile: true, name: 'file1', path: 'dir1/dir2/file1', file: {Name: 'dir1/dir2/file1'}}, + ]}, + ]}, + ]); + expect(pathListToTree(multiplePathsDisjoint)).toEqual([ + {isFile: false, name: 'dir1', path: 'dir1', children: [ + {isFile: false, name: 'dir2', path: 'dir1/dir2', children: [ + {isFile: true, name: 'file1', path: 'dir1/dir2/file1', file: {Name: 'dir1/dir2/file1'}}, + ]}, + ]}, + {isFile: false, name: 'dir3', path: 'dir3', children: [ + {isFile: true, name: 'file2', path: 'dir3/file2', file: {Name: 'dir3/file2'}}, + ]}, + ]); + expect(pathListToTree(multiplePathsShared)).toEqual([ + {isFile: false, name: 'dir1', path: 'dir1', children: [ + {isFile: false, name: 'dir2', path: 'dir1/dir2', children: [ + {isFile: false, name: 'dir3', path: 'dir1/dir2/dir3', children: [ + {isFile: true, name: 'file1', path: 'dir1/dir2/dir3/file1', file: {Name: 'dir1/dir2/dir3/file1'}}, + ]}, + ]}, + {isFile: true, name: 'file2', path: 'dir1/file2', file: {Name: 'dir1/file2'}}, + ]}, + ]); +}); + +const mergeChildWrapper = (testCase: File[]) => { + const tree = pathListToTree(testCase); + mergeChildIfOnlyOneDir(tree); + return tree; +}; + +test('mergeChildIfOnlyOneDir', () => { + expect(mergeChildWrapper(emptyList)).toEqual([]); + expect(mergeChildWrapper(singleFile)).toEqual([ + {isFile: true, name: 'file1', path: 'file1', file: {Name: 'file1'}}, + ]); + expect(mergeChildWrapper(singleDir)).toEqual([ + {isFile: false, name: 'dir1', path: 'dir1', children: [ + {isFile: true, name: 'file1', path: 'dir1/file1', file: {Name: 'dir1/file1'}}, + ]}, + ]); + expect(mergeChildWrapper(nestedDir)).toEqual([ + {isFile: false, name: 'dir1/dir2', path: 'dir1/dir2', children: [ + {isFile: true, name: 'file1', path: 'dir1/dir2/file1', file: {Name: 'dir1/dir2/file1'}}, + ]}, + ]); + expect(mergeChildWrapper(multiplePathsDisjoint)).toEqual([ + {isFile: false, name: 'dir1/dir2', path: 'dir1/dir2', children: [ + {isFile: true, name: 'file1', path: 'dir1/dir2/file1', file: {Name: 'dir1/dir2/file1'}}, + ]}, + {isFile: false, name: 'dir3', path: 'dir3', children: [ + {isFile: true, name: 'file2', path: 'dir3/file2', file: {Name: 'dir3/file2'}}, + ]}, + ]); + expect(mergeChildWrapper(multiplePathsShared)).toEqual([ + {isFile: false, name: 'dir1', path: 'dir1', children: [ + {isFile: false, name: 'dir2/dir3', path: 'dir1/dir2/dir3', children: [ + {isFile: true, name: 'file1', path: 'dir1/dir2/dir3/file1', file: {Name: 'dir1/dir2/dir3/file1'}}, + ]}, + {isFile: true, name: 'file2', path: 'dir1/file2', file: {Name: 'dir1/file2'}}, + ]}, + ]); +}); diff --git a/web_src/js/utils/filetree.ts b/web_src/js/utils/filetree.ts new file mode 100644 index 0000000000..35f9f58189 --- /dev/null +++ b/web_src/js/utils/filetree.ts @@ -0,0 +1,85 @@ +import {dirname, basename} from '../utils.ts'; + +export type FileStatus = 'added' | 'modified' | 'deleted' | 'renamed' | 'copied' | 'typechange'; + +export type File = { + Name: string; + NameHash: string; + Status: FileStatus; + IsViewed: boolean; + IsSubmodule: boolean; +} + +type DirItem = { + isFile: false; + name: string; + path: string; + + children: Item[]; +} + +type FileItem = { + isFile: true; + name: string; + path: string; + file: File; +} + +export type Item = DirItem | FileItem; + +export function pathListToTree(fileEntries: File[]): Item[] { + const pathToItem = new Map<string, DirItem>(); + + // init root node + const root: DirItem = {name: '', path: '', isFile: false, children: []}; + pathToItem.set('', root); + + for (const fileEntry of fileEntries) { + const [parentPath, fileName] = [dirname(fileEntry.Name), basename(fileEntry.Name)]; + + let parentItem = pathToItem.get(parentPath); + if (!parentItem) { + parentItem = constructParents(pathToItem, parentPath); + } + + const fileItem: FileItem = {name: fileName, path: fileEntry.Name, isFile: true, file: fileEntry}; + + parentItem.children.push(fileItem); + } + + return root.children; +} + +function constructParents(pathToItem: Map<string, DirItem>, dirPath: string): DirItem { + const [dirParentPath, dirName] = [dirname(dirPath), basename(dirPath)]; + + let parentItem = pathToItem.get(dirParentPath); + if (!parentItem) { + // if the parent node does not exist, create it + parentItem = constructParents(pathToItem, dirParentPath); + } + + const dirItem: DirItem = {name: dirName, path: dirPath, isFile: false, children: []}; + parentItem.children.push(dirItem); + pathToItem.set(dirPath, dirItem); + + return dirItem; +} + +export function mergeChildIfOnlyOneDir(nodes: Item[]): void { + for (const node of nodes) { + if (node.isFile) { + continue; + } + const dir = node as DirItem; + + mergeChildIfOnlyOneDir(dir.children); + + if (dir.children.length === 1 && dir.children[0].isFile === false) { + const child = dir.children[0]; + dir.name = `${dir.name}/${child.name}`; + dir.path = child.path; + dir.children = child.children; + } + } +} |