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

Refactor file view & render #30227

Merged
merged 1 commit into from
Apr 1, 2024
Merged

Conversation

wxiaoguang
Copy link
Contributor

The old code is inconsistent and fragile, and the UI isn't right.

This PR focuses on refactoring some legacy logic and fix the UI. Some comments are left because it seems to need more time to do deep refactorings in the future.

Before

image

After

image

image

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 1, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 1, 2024
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels Apr 1, 2024
@wxiaoguang wxiaoguang added the backport/v1.22 This PR should be backported to Gitea 1.22 label Apr 1, 2024
Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've thought for a bit about whether it might make sense to extract the too large checks above the switch so that it is always displayed when necessary.
However, since you cannot view binary files anyway, it wouldn't make sense

@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 Apr 1, 2024
@GiteaBot 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 Apr 1, 2024
@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Apr 1, 2024

I've thought for a bit about whether it might make sense to extract the too large checks above the switch so that it is always displayed when necessary. However, since you cannot view binary files anyway, it wouldn't make sense

I have also thought about it. By my understanding, the IsFileTooLarge is only possible to be set to true when IsRepresentableAsText, so I think it is still good enough to move the IsFileTooLarge check above others, it doesn't seem to block other types.

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 1, 2024
@wxiaoguang wxiaoguang merged commit 751997a into go-gitea:main Apr 1, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Apr 1, 2024
@wxiaoguang wxiaoguang deleted the fix-file-size branch April 1, 2024 13:11
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Apr 1, 2024
The old code is inconsistent and fragile, and the UI isn't right.
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Apr 1, 2024
wxiaoguang added a commit that referenced this pull request Apr 1, 2024
Backport #30227 by wxiaoguang

The old code is inconsistent and fragile, and the UI isn't right.

Co-authored-by: wxiaoguang <[email protected]>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 3, 2024
* giteaofficial/main:
  Refactor "dump" sub-command (go-gitea#30240)
  Add -u git to docs when using docker exec with root installation (go-gitea#29314)
  Show 12 lines in markup code preview (go-gitea#30255)
  Fixes go-gitea#27605: inline math blocks can't be preceeded/followed by alphanumerical characters (go-gitea#30175)
  Render embedded code preview by permlink in markdown (go-gitea#30234)
  Fix missing 0 prefix of GPG key id (go-gitea#30245)
  Fix spacing in issue navbar (go-gitea#30238)
  Add unique index for project_issue to prevent duplicate data (go-gitea#30190)
  [skip ci] Updated translations via Crowdin
  Refactor commit signature parser (go-gitea#30228)
  Refactor dropzone (go-gitea#30232)
  Remove scheduled action tasks if the repo is archived (go-gitea#30224)
  Refactor file view & render (go-gitea#30227)
  Refactor DeleteInactiveUsers, fix bug and add tests (go-gitea#30206)
  [skip ci] Updated licenses and gitignores
  Add `/options/license` and `/options/gitignore` to `.ignore` (go-gitea#30219)
AvengerMoJo pushed a commit to AvengerMoJo/gitea that referenced this pull request Apr 8, 2024
The old code is inconsistent and fragile, and the UI isn't right.
@wxiaoguang wxiaoguang added type/bug skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Apr 27, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created backport/v1.22 This PR should be backported to Gitea 1.22 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files size/M Denotes a PR that changes 30-99 lines, ignoring generated files. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants