Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 28, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -2595,7 +2595,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
Expand Down
12 changes: 12 additions & 0 deletions routers/web/repo/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.Data["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)
Expand Down
10 changes: 10 additions & 0 deletions routers/web/repo/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.Data["DiffFiles"] = transformDiffTreeForUI(diffTree, nil)
}

headCommit, err := ci.HeadGitRepo.GetCommit(headCommitID)
if err != nil {
ctx.ServerError("GetCommit", err)
Expand Down
23 changes: 23 additions & 0 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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.Data["DiffFiles"] = transformDiffTreeForUI(diffTree, filesViewedState)
}

ctx.Data["Diff"] = diff
ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0

Expand Down
32 changes: 32 additions & 0 deletions routers/web/repo/treelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
10 changes: 3 additions & 7 deletions templates/repo/diff/box.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,15 @@
</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 diffDataFiles = [{{range $i, $file := .DiffFiles}}
{Name:"{{$file.Name}}",NameHash:"{{$file.NameHash}}",Status:{{$file.Status}},IsSubmodule:{{$file.IsSubmodule}},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,
Expand Down
1 change: 0 additions & 1 deletion templates/repo/diff/options_dropdown.tmpl
Original file line number Diff line number Diff line change
@@ -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>
Expand Down
60 changes: 0 additions & 60 deletions web_src/js/components/DiffFileList.vue

This file was deleted.

72 changes: 4 additions & 68 deletions web_src/js/components/DiffFileTree.vue
Original file line number Diff line number Diff line change
@@ -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;
});

Expand Down Expand Up @@ -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>

Expand Down
Loading
Loading