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

Fix path escaping in 'Find files' #30025

Closed
wants to merge 1 commit into from
Closed

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Mar 23, 2024

Fixes: #30020

Before:
Screenshot 2024-03-23 at 13 10 07

After:
image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 23, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 23, 2024
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Mar 23, 2024
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Sorry , it is not right ... I will propose a new fix.

-> Escape paths for find file correctly #30026

@silverwind
Copy link
Member Author

It is right and exactly the same outcome as yours.

@silverwind
Copy link
Member Author

But yes, I agree it's better done in go code instead of templates.

@silverwind silverwind closed this Mar 23, 2024
@silverwind silverwind deleted the findf branch March 23, 2024 12:19
@wxiaoguang
Copy link
Contributor

It is right and exactly the same outcome as yours.

Outcome is the same doesn't mean the underlying logic is right.

For this case: the "DataLink" is a "Link" from backend, so it should be a fully escaped and 100% correct URL link. It shouldn't be "escaped" again. Otherwise, it would cause double-escaping in the future.

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jun 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/templates This PR modifies the template files size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Go to file" doesn't work for branches containing a #
3 participants