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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

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

wants to merge 2 commits into from

Conversation

McRaeAlex
Copy link
Contributor

@McRaeAlex McRaeAlex commented Feb 6, 2025 •
edited
Loading

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

@GiteaBot 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 pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 6, 2025
@github-actions 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 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 McRaeAlex force-pushed the gitea/am/file-tab-diff-tree branch from 4799444 to 5752410 Compare February 7, 2025 03:31
@pull-request-size 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
Copy link
Contributor Author

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

@McRaeAlex McRaeAlex marked this pull request as ready for review February 7, 2025 03:38
@lunny lunny requested review from silverwind and kerwin612 February 7, 2025 03:45
@GiteaBot 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 lunny requested a review from wxiaoguang February 7, 2025 07:59
@lunny
Copy link
Member

lunny commented Feb 7, 2025

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

@lunny lunny added this to the 1.24.0 milestone Feb 7, 2025
@McRaeAlex
Copy link
Contributor Author

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
Copy link
Member

lunny commented Feb 7, 2025

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.

@McRaeAlex
Modify Diff View FileTree to show all files
64dac90 
== Changes

* removes Show Status button on diff
* uses `git diff-tree` to generate the file tree for the diff
@McRaeAlex
update locale to remove diff.show_diff_stats
0982492
@McRaeAlex McRaeAlex force-pushed the gitea/am/file-tab-diff-tree branch from b50105b to 0982492 Compare February 8, 2025 00:27
@lunny
Copy link
Member

lunny commented Feb 8, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kerwin612 kerwin612 kerwin612 approved these changes

@silverwind silverwind Awaiting requested review from silverwind

@wxiaoguang wxiaoguang Awaiting requested review from wxiaoguang

Assignees
No one assigned
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/frontend modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/translation size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
1.24.0
Development

Successfully merging this pull request may close these issues.
4 participants
@McRaeAlex @lunny @kerwin612 @GiteaBot