Skip to content
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

Improve implementation of diff-file-tree #32768

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
85 changes: 85 additions & 0 deletions services/gitdiff/gitdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,12 +448,20 @@ func getCommitFileLineCount(commit *git.Commit, filePath string) int {
return lineCount
}

type FileTreeNode struct {
IsFile bool
Name string
File *DiffFile
Children []*FileTreeNode
}

// Diff represents a difference between two git trees.
type Diff struct {
Start, End string
NumFiles int
TotalAddition, TotalDeletion int
Files []*DiffFile
FileTree []*FileTreeNode
IsIncomplete bool
NumViewedFiles int // user-specific
}
Expand Down Expand Up @@ -1212,6 +1220,8 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi
}
}

diff.FileTree = buildTree(diff.Files)

if opts.FileOnly {
return diff, nil
}
Expand Down Expand Up @@ -1384,3 +1394,78 @@ func GetWhitespaceFlag(whitespaceBehavior string) git.TrustedCmdArgs {
log.Warn("unknown whitespace behavior: %q, default to 'show-all'", whitespaceBehavior)
return nil
}

func buildTree(files []*DiffFile) []*FileTreeNode {
kerwin612 marked this conversation as resolved.
Show resolved Hide resolved
result := make(map[string]*FileTreeNode)
for _, file := range files {
splits := strings.Split(file.Name, "/")
currentNode := &FileTreeNode{Name: splits[0], IsFile: false}
if _, exists := result[splits[0]]; !exists {
result[splits[0]] = currentNode
} else {
currentNode = result[splits[0]]
}

parent := currentNode
for _, split := range splits[1:] {
found := false
for _, child := range parent.Children {
if child.Name == split {
parent = child
found = true
break
}
}
if !found {
newNode := &FileTreeNode{Name: split, IsFile: false}
parent.Children = append(parent.Children, newNode)
parent = newNode
}
}

lastNode := parent
lastNode.IsFile = true
lastNode.File = file
}

var roots []*FileTreeNode
for _, node := range result {
if len(node.Children) > 0 {
mergedNode := mergeSingleChildDirs(node)
sortChildren(mergedNode)
roots = append(roots, mergedNode)
} else {
roots = append(roots, node)
}
}
sortChildren(&FileTreeNode{Children: roots})
return roots
}

func mergeSingleChildDirs(node *FileTreeNode) *FileTreeNode {
if len(node.Children) == 1 && !node.Children[0].IsFile {
merged := &FileTreeNode{
Name: fmt.Sprintf("%s/%s", node.Name, node.Children[0].Name),
Children: node.Children[0].Children,
IsFile: node.Children[0].IsFile,
File: node.Children[0].File,
}
return mergeSingleChildDirs(merged)
}
for i, child := range node.Children {
node.Children[i] = mergeSingleChildDirs(child)
}
return node
}

func sortChildren(node *FileTreeNode) {
sort.Slice(node.Children, func(i, j int) bool {
if node.Children[i].IsFile == node.Children[j].IsFile {
return node.Children[i].Name < node.Children[j].Name
}
return !node.Children[i].IsFile
kerwin612 marked this conversation as resolved.
Show resolved Hide resolved
})
for _, child := range node.Children {
sortChildren(child)
}
}
188 changes: 188 additions & 0 deletions services/gitdiff/gitdiff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,3 +668,191 @@ func TestNoCrashes(t *testing.T) {
ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), "")
}
}

func TestBuildTree(t *testing.T) {
tests := []struct {
name string
input []*DiffFile
expected []*FileTreeNode
}{
{
name: "Test case 1",
input: []*DiffFile{
{Name: "test1", NameHash: "b444ac06613fc8d63795be9ad0beaf55011936ac"},
{Name: "2/2", NameHash: "23b3d60a69a4b1bacb1a3b9ced0a8ac609efbf61"},
},
expected: []*FileTreeNode{
{
Name: "2",
IsFile: false,
Children: []*FileTreeNode{
{
Name: "2",
IsFile: true,
File: &DiffFile{
Name: "2/2",
NameHash: "23b3d60a69a4b1bacb1a3b9ced0a8ac609efbf61",
},
},
},
},
{
Name: "test1",
IsFile: true,
File: &DiffFile{
Name: "test1",
NameHash: "b444ac06613fc8d63795be9ad0beaf55011936ac",
},
},
},
},
{
name: "Test case 2",
input: []*DiffFile{
{Name: "a/b/d", NameHash: "hash2"},
{Name: "a/e", NameHash: "hash3"},
{Name: "a/b/c", NameHash: "hash1"},
{Name: "f", NameHash: "hash4"},
},
expected: []*FileTreeNode{
{
Name: "a",
IsFile: false,
Children: []*FileTreeNode{
{
Name: "b",
IsFile: false,
Children: []*FileTreeNode{
{
Name: "c",
IsFile: true,
File: &DiffFile{
Name: "a/b/c",
NameHash: "hash1",
},
},
{
Name: "d",
IsFile: true,
File: &DiffFile{
Name: "a/b/d",
NameHash: "hash2",
},
},
},
},
{
Name: "e",
IsFile: true,
File: &DiffFile{
Name: "a/e",
NameHash: "hash3",
},
},
},
},
{
Name: "f",
IsFile: true,
File: &DiffFile{
Name: "f",
NameHash: "hash4",
},
},
},
},
{
name: "Test case 3",
input: []*DiffFile{
{Name: "dir2/file4", NameHash: "hash4"},
{Name: "dir1/file1", NameHash: "hash1"},
{Name: "dir2/file3", NameHash: "hash3"},
{Name: "dir1/file2", NameHash: "hash2"},
{Name: "file5", NameHash: "hash5"},
},
expected: []*FileTreeNode{
{
Name: "dir1",
IsFile: false,
Children: []*FileTreeNode{
{
Name: "file1",
IsFile: true,
File: &DiffFile{
Name: "dir1/file1",
NameHash: "hash1",
},
},
{
Name: "file2",
IsFile: true,
File: &DiffFile{
Name: "dir1/file2",
NameHash: "hash2",
},
},
},
},
{
Name: "dir2",
IsFile: false,
Children: []*FileTreeNode{
{
Name: "file3",
IsFile: true,
File: &DiffFile{
Name: "dir2/file3",
NameHash: "hash3",
},
},
{
Name: "file4",
IsFile: true,
File: &DiffFile{
Name: "dir2/file4",
NameHash: "hash4",
},
},
},
},
{
Name: "file5",
IsFile: true,
File: &DiffFile{
Name: "file5",
NameHash: "hash5",
},
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := buildTree(tt.input)
if !compareFileTreeNodes(result, tt.expected) {
t.Errorf("Expected %v, but got %v", tt.expected, result)
}
})
}
}

func compareFileTreeNodes(a, b []*FileTreeNode) bool {
if len(a) != len(b) {
return false
}
for i := range a {
if a[i].Name != b[i].Name || a[i].IsFile != b[i].IsFile {
return false
}
if a[i].IsFile && b[i].IsFile {
if a[i].File.Name != b[i].File.Name || a[i].File.NameHash != b[i].File.NameHash {
return false
}
}
if !compareFileTreeNodes(a[i].Children, b[i].Children) {
return false
}
}
return true
}
10 changes: 2 additions & 8 deletions templates/repo/diff/box.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -61,35 +61,29 @@
</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 = {
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}}
<div id="diff-file-tree" class="tw-hidden not-mobile"></div>
{{template "repo/diff/file_tree" dict "Files" .Diff.FileTree "IsIncomplete" .Diff.IsIncomplete "LoadMoreLink" (printf "?skip-to=%s&file-only=true" .Diff.End)}}
<script>
if (diffTreeVisible) document.getElementById('diff-file-tree').classList.remove('tw-hidden');
</script>
Expand Down Expand Up @@ -228,7 +222,7 @@
<div class="diff-file-box diff-box file-content tw-mt-2" id="diff-incomplete">
<h4 class="ui top attached header tw-font-normal tw-flex tw-items-center tw-justify-between">
{{ctx.Locale.Tr "repo.diff.too_many_files"}}
<a class="ui basic tiny button" id="diff-show-more-files" data-href="?skip-to={{.Diff.End}}&file-only=true">{{ctx.Locale.Tr "repo.diff.show_more"}}</a>
<a class="ui basic tiny button diff-show-more-files" data-href="?skip-to={{.Diff.End}}&file-only=true">{{ctx.Locale.Tr "repo.diff.show_more"}}</a>
</h4>
</div>
{{end}}
Expand Down
12 changes: 12 additions & 0 deletions templates/repo/diff/file_tree.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<div id="diff-file-tree" class="file-tree tw-hidden not-mobile">
<div class="file-tree-items">
{{range .Files}}
{{template "repo/diff/file_tree_item" .}}
{{end}}
</div>
{{if .IsIncomplete}}
<div class="tw-pt-1">
<a class="ui basic tiny button diff-show-more-files" data-href="{{.LoadMoreLink}}">{{ctx.Locale.Tr "repo.diff.show_more"}}</a>
</div>
{{end}}
</div>
34 changes: 34 additions & 0 deletions templates/repo/diff/file_tree_item.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{{if .IsFile}}
<a class="item-file {{if .File.IsViewed}} viewed {{end}}" title="{{.Name}}" href="#diff-{{.File.NameHash}}">
<!-- file -->
{{svg "octicon-file"}}
<span class="gt-ellipsis tw-flex-1">{{.Name}}</span>
{{if eq .File.Type 1}}
{{svg "octicon-diff-added" 16 "text green"}}
{{else if eq .File.Type 2}}
{{svg "octicon-diff-modified" 16 "text yellow"}}
{{else if eq .File.Type 3}}
{{svg "octicon-diff-removed" 16 "text red"}}
{{else if eq .File.Type 4}}
{{svg "octicon-diff-renamed" 16 "text teal"}}
{{else if eq .File.Type 5}}
{{svg "octicon-diff-renamed" 16 "text green"}}
{{end}}
</a>
{{else}}
<div class="item-directory" title="{{.Name}}">
<!-- directory -->
{{svg "octicon-chevron-right" 16 "tw-hidden"}}
{{svg "octicon-chevron-down" 16}}
{{svg "octicon-file-directory-fill" 16 "text primary tw-hidden"}}
{{svg "octicon-file-directory-open-fill" 16 "text primary"}}
<span class="gt-ellipsis">{{.Name}}</span>
</div>
{{end}}
{{if and .Children (gt (len .Children) 0)}}
<div class="sub-items">
{{range .Children}}
{{template "repo/diff/file_tree_item" .}}
{{end}}
</div>
{{end}}
Loading
Loading