Skip to content

Conversation

@silverwind
Copy link
Member

@silverwind silverwind commented Dec 15, 2025

This makes the threshold value passed to git diff --find-renames configurable. Git's default is 50% and this sets the same value. For example, here is a file rename and change with 3 out of 4 changed lines with a value of 25%:

image

The reason for this change is the rewrite+rename in #36118 and I checked the similarity index there, and it's only 21%, so will need a value of 20% or less:

diff --git a/modules/storage/s3.go b/modules/storage/minio.go
similarity index 21%

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 15, 2025
@github-actions github-actions bot added modifies/go Pull requests that update Go code docs-update-needed The document needs to be updated synchronously labels Dec 15, 2025
@silverwind silverwind changed the title Add git.DIFF_RENAME_THRESHOLD option Add git.DIFF_RENAME_SIMILARITY_THRESHOLD option Dec 15, 2025
@silverwind
Copy link
Member Author

silverwind commented Dec 15, 2025

Maybe 25% is too prone to false-positive detections. Maybe I will revert to 50%, which is a safe default and I see why git has chosen it.

@silverwind
Copy link
Member Author

Reverted to 50%. Ideally I think this could be a UI option, but it being a server option will suffice initially.

Copy link
Contributor

@TheFox0x7 TheFox0x7 left a comment

Choose a reason for hiding this comment

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

Not sure if that's a concern given the existing issues with settings but shouldn't settings value be checked? Otherwise git will error out later at runtime

@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 Dec 15, 2025
@silverwind
Copy link
Member Author

Hmm I guess we can add a small validation like ^([0-9]|[1-9][0-9]|100)%$ to allow only integers between 0 and 100.

@silverwind
Copy link
Member Author

Validation added.

@silverwind
Copy link
Member Author

silverwind commented Dec 15, 2025

PS: I also checked git diff config and it appears there is no config option to alter the 50% default, so I assume it can only be passed via CLI, so having this option in gitea is valuable.

@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 Dec 17, 2025
@wxiaoguang wxiaoguang added the type/enhancement An improvement of existing functionality label Dec 17, 2025
@wxiaoguang wxiaoguang added this to the 1.26.0 milestone Dec 17, 2025
@wxiaoguang wxiaoguang enabled auto-merge (squash) December 17, 2025 09:41
@wxiaoguang wxiaoguang merged commit 852bf5e into go-gitea:main Dec 17, 2025
23 checks passed
@silverwind silverwind deleted the rename branch December 17, 2025 10:34
@silverwind
Copy link
Member Author

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

Labels

docs-update-needed The document needs to be updated synchronously 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 type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants