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 1 commit into
base: main
Choose a base branch
from
Open

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

wants to merge 1 commit 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 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.

@McRaeAlex
Copy link
Contributor Author

@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

@McRaeAlex McRaeAlex force-pushed the gitea/am/file-tab-diff-tree branch from 0982492 to 705f380 Compare February 24, 2025 22:00
delvh
delvh reviewed Feb 24, 2025
View reviewed changes
@McRaeAlex McRaeAlex force-pushed the gitea/am/file-tab-diff-tree branch from 705f380 to dbd2c36 Compare February 25, 2025 00:18
@silverwind silverwind mentioned this pull request Feb 25, 2025
Merged
silverwind added a commit that referenced this pull request Feb 25, 2025
@silverwind
Enable @typescript-eslint/no-use-before-define (#33715)
b35a9da 
The [rule](https://typescript-eslint.io/rules/no-use-before-define/) is
a superset of the eslint base rule, and I tested it with
#33514 (comment)
where it does not raise an error for cyclic types.
@McRaeAlex
Modify Diff View FileTree to show all files
4e74e74 
== Changes

* removes Show Status button on diff
* uses `git diff-tree` to generate the file tree for the diff

update locale to remove diff.show_diff_stats
@McRaeAlex McRaeAlex force-pushed the gitea/am/file-tab-diff-tree branch from dbd2c36 to 4e74e74 Compare February 25, 2025 19:28
@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 25, 2025
@McRaeAlex McRaeAlex requested a review from silverwind February 25, 2025 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

@delvh delvh delvh left review comments

@kerwin612 kerwin612 kerwin612 approved these changes

@silverwind silverwind Awaiting requested review from silverwind

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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
1.24.0
Development

Successfully merging this pull request may close these issues.
7 participants
@McRaeAlex @lunny @silverwind @wxiaoguang @kerwin612 @delvh @GiteaBot