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

McRaeAlex · 2025-02-06T03:40:44Z

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.

Screen.Recording.2025-02-06.at.6.51.03.PM.mov

McRaeAlex · 2025-02-07T03:38:46Z

@delvh @lunny I taken this out of draft so we can discuss the open question and any other behaviour you want to go over.

templates/repo/diff/options_dropdown.tmpl

lunny · 2025-02-07T18:56:25Z

I think you don't need to update all the language files except en_US.

McRaeAlex · 2025-02-07T19:21:09Z

I think you don't need to update all the language files except en_US.

I am not sure I understand, If I remove it from en_US then why wouldn't I also remove it from every other language?

lunny · 2025-02-07T20:27:21Z

I think you don't need to update all the language files except en_US.

I am not sure I understand, If I remove it from en_US then why wouldn't I also remove it from every other language?

Because other language files will be synced from translate.gitea.com, so basicaly you just need to change the en_US file.

lunny · 2025-02-08T20:02:24Z

I did a test manually, looks like a modified file can not be selected.

McRaeAlex · 2025-02-24T21:48:45Z

@lunny Apologizes for the delay.

I wasn't able to reproduce your issue. Could you provide some more details?

Screen.Recording.2025-02-24.at.1.47.47.PM.mov

templates/repo/diff/options_dropdown.tmpl

web_src/js/components/DiffFileTree.vue

routers/web/repo/pull.go

web_src/js/components/DiffFileTreeItem.vue

web_src/js/features/repo-diff.ts

wxiaoguang · 2025-02-26T00:07:24Z

Still have question about the script block. Are these variables used? Is the comment correct?

wxiaoguang · 2025-02-26T00:08:42Z

ps: no need to force-push when review starts, the final merge is a squash merge. Force-push makes the review difficult because every-line needs to be re-reviewed since everything is in a commit.

routers/web/repo/treelist.go

McRaeAlex · 2025-02-26T01:47:55Z

ps: no need to force-push when review starts, the final merge is a squash merge. Force-push makes the review difficult because every-line needs to be re-reviewed since everything is in a commit.

I use jj which makes this behaviour automatic and keeps my "stacked" changes up to date. I can switch to using the git cli and adding commits if its preferred.

Still have question about the script block. Are these variables used? Is the comment correct?

I have updated the comment. window.config.pageData.diffFileInfo is used in web_src/js/modules/stores.ts. I cannot find any place where diffData is used (other than diffData.isIncomplete) for example diffData.statisticsMessage seems to be dead code, I have removed it.

routers/web/repo/treelist.go

web_src/js/components/DiffFileTreeItem.vue

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

silverwind · 2025-02-27T09:36:48Z

templates/repo/diff/box.tmpl

-			diffFileInfo.files.push(...diffDataFiles);
-			window.config.pageData.diffFileInfo = diffFileInfo;
-		</script>
-		<div id="diff-file-list"></div>


With this removal, will it not flash wrong content on page load when diff tree is visible?

Ah I may be misunderstanding and this was just some data modification that has no reason to be in templates.

Yes, it's not related to "content flash" (and this block was marked as "module", it only executes after DOM ready).

Some old PRs used very tricky methods to make the Vue work, now it's the time to make code overall right.

Yeah we should remove all such template js, except those cases that serve the purpose of avoiding flash, but those are better migrated to web components.

* giteaofficial/main: Use pullrequestlist instead of []*pullrequest (go-gitea#33765) Upgrade act to 0.261.4 and actions-proto-go to v0.4.1 (go-gitea#33760) Webhook add X-Gitea-Hook-Installation-Target-Type Header (go-gitea#33752) Fix dynamic content loading init problem (go-gitea#33748) [skip ci] Updated translations via Crowdin Add composor source field (go-gitea#33502) upgrade go-crypto from 1.1.5 to 1.1.6 (go-gitea#33745) Disable go license generation as part of `make tidy` (go-gitea#33747) Refactor repo-diff.ts (go-gitea#33746) Use `git diff-tree` for `DiffFileTree` on diff pages (go-gitea#33514) [skip ci] Updated translations via Crowdin Improve "generate new access token" form (go-gitea#33730) Remove superflous tw-content-center (go-gitea#33741) Clone repository with Tea CLI (go-gitea#33725) allow filtering /repos/{owner}/{repo}/pulls by target base branch queryparam (go-gitea#33684) Show info about maintainers are allowed to edit a PR (go-gitea#33738) Improve admin user view page (go-gitea#33735) [skip ci] Updated translations via Crowdin Align sidebar gears to the right (go-gitea#33721)

wxiaoguang · 2025-03-06T04:59:57Z

Found a bug:

Now the file tree doesn't follow the selected commit:

Fixes #33514 (comment) Co-authored-by: wxiaoguang <[email protected]>

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 6, 2025

pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 6, 2025

github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/frontend labels Feb 6, 2025

McRaeAlex changed the title ~~[WIP] Stacked on https://github.com/go-gitea/gitea/pull/33369~~ Use git diff-tree for DiffFileTree on diff pages Feb 7, 2025

McRaeAlex force-pushed the gitea/am/file-tab-diff-tree branch from 4799444 to 5752410 Compare February 7, 2025 03:31

pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 7, 2025

McRaeAlex marked this pull request as ready for review February 7, 2025 03:38

lunny requested review from silverwind and kerwin612 February 7, 2025 03:45

kerwin612 approved these changes Feb 7, 2025

View reviewed changes

templates/repo/diff/options_dropdown.tmpl Show resolved Hide resolved

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 7, 2025

lunny requested a review from wxiaoguang February 7, 2025 07:59

github-actions bot added the modifies/translation label Feb 7, 2025

lunny added this to the 1.24.0 milestone Feb 7, 2025

McRaeAlex force-pushed the gitea/am/file-tab-diff-tree branch from b50105b to 0982492 Compare February 8, 2025 00:27

McRaeAlex force-pushed the gitea/am/file-tab-diff-tree branch from 0982492 to 705f380 Compare February 24, 2025 22:00

delvh reviewed Feb 24, 2025

View reviewed changes

McRaeAlex force-pushed the gitea/am/file-tab-diff-tree branch from 705f380 to dbd2c36 Compare February 25, 2025 00:18

wxiaoguang reviewed Feb 25, 2025

View reviewed changes

web_src/js/features/repo-diff.ts Show resolved Hide resolved

wxiaoguang reviewed Feb 25, 2025

View reviewed changes

web_src/js/features/repo-diff.ts Outdated Show resolved Hide resolved

McRaeAlex requested a review from silverwind February 25, 2025 19:58

silverwind approved these changes Feb 25, 2025

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 25, 2025

wxiaoguang reviewed Feb 26, 2025

View reviewed changes

routers/web/repo/treelist.go Outdated Show resolved Hide resolved

wxiaoguang reviewed Feb 26, 2025

View reviewed changes

routers/web/repo/treelist.go Show resolved Hide resolved

wxiaoguang reviewed Feb 26, 2025

View reviewed changes

routers/web/repo/treelist.go Outdated Show resolved Hide resolved

Clean up code to address review comments

3580286

wxiaoguang reviewed Feb 26, 2025

View reviewed changes

routers/web/repo/treelist.go Outdated Show resolved Hide resolved

removed path() from DiffTreeRecord

5619691

silverwind reviewed Feb 26, 2025

View reviewed changes

web_src/js/components/DiffFileTreeItem.vue Outdated Show resolved Hide resolved

McRaeAlex and others added 3 commits February 26, 2025 10:06

Update web_src/js/components/DiffFileTreeItem.vue

b1ba89d

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

remove legacy script block and simplify loadUntilFound

7adbb76

add missing return

adf5f3c

silverwind reviewed Feb 27, 2025

View reviewed changes

Merge branch 'main' into gitea/am/file-tab-diff-tree

3995ef5

wxiaoguang approved these changes Feb 28, 2025

View reviewed changes

silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 28, 2025

silverwind enabled auto-merge (squash) February 28, 2025 00:22

Merge branch 'main' into gitea/am/file-tab-diff-tree

a4d3bf4

silverwind merged commit aba96f6 into go-gitea:main Feb 28, 2025
26 checks passed

GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 28, 2025

McRaeAlex deleted the gitea/am/file-tab-diff-tree branch February 28, 2025 16:11

McRaeAlex mentioned this pull request Mar 6, 2025

[Bug] Use correct start and end commits for GetDiffTree #33816

Merged

wxiaoguang added a commit that referenced this pull request Mar 7, 2025

Use correct start and end commits for GetDiffTree (#33816)

582ad33

Fixes #33514 (comment) Co-authored-by: wxiaoguang <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

McRaeAlex commented Feb 6, 2025 •

edited

Loading

McRaeAlex commented Feb 7, 2025

lunny commented Feb 7, 2025

McRaeAlex commented Feb 7, 2025

lunny commented Feb 7, 2025

lunny commented Feb 8, 2025

McRaeAlex commented Feb 24, 2025

wxiaoguang commented Feb 26, 2025

wxiaoguang commented Feb 26, 2025

McRaeAlex commented Feb 26, 2025

silverwind Feb 27, 2025

silverwind Feb 27, 2025

wxiaoguang Feb 27, 2025

silverwind Feb 27, 2025

wxiaoguang commented Mar 6, 2025

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

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

Conversation

McRaeAlex commented Feb 6, 2025 • edited Loading

Changes

Open Questions

Demo

McRaeAlex commented Feb 7, 2025

lunny commented Feb 7, 2025

McRaeAlex commented Feb 7, 2025

lunny commented Feb 7, 2025

lunny commented Feb 8, 2025

McRaeAlex commented Feb 24, 2025

wxiaoguang commented Feb 26, 2025

wxiaoguang commented Feb 26, 2025

McRaeAlex commented Feb 26, 2025

silverwind Feb 27, 2025

Choose a reason for hiding this comment

silverwind Feb 27, 2025

Choose a reason for hiding this comment

wxiaoguang Feb 27, 2025

Choose a reason for hiding this comment

silverwind Feb 27, 2025

Choose a reason for hiding this comment

wxiaoguang commented Mar 6, 2025

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

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

McRaeAlex commented Feb 6, 2025 •

edited

Loading