Skip to content

Conversation

surya-purohit
Copy link
Contributor

shows the main LFS filesize instead of the pointer filesize when viewing a file

shows the main LFS filesize instead of the pointer filesize when viewing a file
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 16, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Oct 16, 2025
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Oct 16, 2025

Thank you for the PR, but I think we shouldn't use a quick patch for this requirement.

All existing code assumes that "fileSize is the size of blob in the commit", if you changed this assumption, it would cause bugs.

The first rule: do not change widely used definition in existing code unless you are pretty sure it is safe.


Update: the change is right, need to revert to the old behavior

@wxiaoguang wxiaoguang marked this pull request as draft October 16, 2025 05:43
@surya-purohit
Copy link
Contributor Author

surya-purohit commented Oct 16, 2025

Thank you for the PR, but I think we shouldn't use a quick patch for this requirement.

All existing code assumes that "fileSize is the size of blob in the commit", if you changed this assumption, it would cause bugs.

The first rule: do not change widely used definition in existing code unless you are pretty sure it is safe.

I understand your point — it makes sense not to change a widely used definition without ensuring it’s safe.
It was working the same way in v1.24.4, and the View File UI displays the size of the actual LFS file rather than the pointer, which is why I approached it this way.
Perhaps we can add a check here — if it’s an LFS file, show the LFS file size; otherwise, show the regular file size.

@wxiaoguang
Copy link
Contributor

It was working the same way in v1.24.4, and the View File UI displays the size of the actual LFS file rather than the pointer, which is why I approached it this way.

Hmm, you are right. Related to Add support for 3D/CAD file formats preview (#34794)

In old code, fileSize does sometimes mean blob size, sometimes mean LFS size. So maybe it's safe to revert the old behavior. Will take a look again.

@wxiaoguang
Copy link
Contributor

Made a small change in bd02218 , use a new name blobOrLfsSize, then it seems clearer now 😄

@wxiaoguang wxiaoguang changed the title 🐛 (lfs) fixes filesize of the LFS file Use LFS object size instead of blob size when viewing a LFS file Oct 16, 2025
@wxiaoguang wxiaoguang marked this pull request as ready for review October 16, 2025 06:26
@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 Oct 16, 2025
@wxiaoguang wxiaoguang added this to the 1.26.0 milestone Oct 16, 2025
@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 Oct 16, 2025
@github-actions github-actions bot added modifies/templates This PR modifies the template files modifies/frontend labels Oct 16, 2025
@wxiaoguang
Copy link
Contributor

And added more tests in 7ddf3b2 , the behavior should be stable now.

@wxiaoguang wxiaoguang merged commit bf8ecf7 into go-gitea:main Oct 16, 2025
26 checks passed
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Oct 16, 2025
…gitea#35679)

shows the main LFS filesize instead of the pointer filesize when viewing
a file

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Oct 16, 2025
wxiaoguang added a commit that referenced this pull request Oct 16, 2025
) (#35680)

Backport #35679 by surya-purohit

shows the main LFS filesize instead of the pointer filesize when viewing
a file

Co-authored-by: Surya Purohit <suryaprakash.sharma@sourcefuse.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 20, 2025
* giteaofficial/main:
  Avoid emoji mismatch and allow to only enable chosen emojis (go-gitea#35692)
  feat(diff): Enable commenting on expanded lines in PR diffs (go-gitea#35662)
  Fix various bugs (go-gitea#35684)
  Fix workflow run event status while rerunning a failed job (go-gitea#35689)
  Use gitrepo.Repository instead of wikipath (go-gitea#35398)
  [skip ci] Updated translations via Crowdin
  Bump `actions/labeler` to v6 (go-gitea#35681)
  Use LFS object size instead of blob size when viewing a LFS file (go-gitea#35679)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/done All backports for this PR have been created backport/v1.25 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/frontend modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants