Thank you for the update, but I think we should avoid doing that .....

Sprintf("%s/%s/%s/src/%s", AppSubURL, PathEscape(username), PathEscape(reponame), PathEscapeSegments(*)) should be good enough and it's not worth to test it because it has been widely used everywhere. We can just keep code simple.

If you don't mind, I can help to make some improvements.

Thank you for the update, but I think we should avoid doing that .....

Sprintf("%s/%s/%s/src/%s", AppSubURL, PathEscape(username), PathEscape(reponame), PathEscapeSegments(*)) should be good enough and it's not worth to test it because it has been widely used everywhere. We can just keep code simple.

If you don't mind, I can help to make some improvements.

Was in the process of responding to that one. I'm happy to make the change if you want, but before usingpaths.Join I looked through the existing code used to construct redirects to ensure the style was similar. It looked like there was a preference for using path.Join instead of fmt.Sprintf. (example).

If you still want me to replace it with fmt.Sprintf, I can.

It looked like there was a preference for using path.Join instead of fmt.Sprintf. (example).

That's the only (maybe a few) legacy usages. Most other code doesn't usepath.Join.

Actually, you could also just useRepoLink here since you are in aRepoAssigment context.

Hmm, one more thing, I think the use of/tree/{CommitID} is wrong.

Because it redirects and routes to the deprecated handler/src/{CommitID} if I understand correctly.

Actually, you could also just useRepoLink here since you are in aRepoAssigment context.

Used this method in92cb2a8

Because it redirects and routes to the deprecated handler /src/{CommitID} if I understand correctly.

This is correct, but intentional. Github's URL structure is<repo>/tree/<ref> not<repo>/tree/<sha>. A couple of examples:

• https://github.com/go-gitea/gitea/tree/main
• https://github.com/go-gitea/gitea/tree/a89c73530327784ce99e1e07c269ea8ff1861fdc

When hitting this in Gitea, the user will have two separate redirects to get to the canonical URL. The flow would look like this:

• https://gitea.com/gitea/gitea-mirror/tree/a89c73530327784ce99e1e07c269ea8ff1861fdc
• https://gitea.com/gitea/gitea-mirror/src/a89c73530327784ce99e1e07c269ea8ff1861fdc
• https://gitea.com/gitea/gitea-mirror/src/commit/a89c73530327784ce99e1e07c269ea8ff1861fdc

Then it comes to this question ....#33424 (comment)

Why we should support such/tree/ path? What's the benefit?

Then it comes to this question ....#33424 (comment)

Why we should support such/tree/ path? What's the benefit?

The intent of this PR is so that when a user clicks on a submodule commit, either in the repo home view, or when adding a new submodule in a diff, that shows them the tree instead of the commit.

Right now if I add a submodule ofgitea.com/gitea/gitea-mirror@a89c73530327784ce99e1e07c269ea8ff1861fdc, it links tohttps://gitea.com/gitea/gitea-mirror/commit/a89c73530327784ce99e1e07c269ea8ff1861fdc. This updates that to show (after redirects)https://gitea.com/gitea/gitea-mirror/src/commit/a89c73530327784ce99e1e07c269ea8ff1861fdc, allowing them to browse the repo at that commit.

The issue of adding a/tree/ path comes in for submodules that are not hosted on Gitea. If I were to link out tohttps://github.com/go-gitea/gitea/src/commit/a89c73530327784ce99e1e07c269ea8ff1861fdc, that results in a 404 for the user, who would need to navigate to the repo, and then search for the commit. That content is instead available athttps://github.com/go-gitea/gitea/tree/a89c73530327784ce99e1e07c269ea8ff1861fdc.

We could try to guess what type of forge a given submodule links out to, and then either usesrc/commit for Gitea ortree for others, but self-hosted deployments would make that error-prone. Adding a redirect for thesetree paths allows using the same link across Gitea and other forges.

Thank you for the explanation, it's clear to me now.

I think we need to add the background information to comments, otherwise the future readers would still have the same question: why/tree/ is used ....

Thank you for the explanation, it's clear to me now.

I think we need to add the background information to comments, otherwise the future readers would still have the same question: why/tree/ is used ....

I updated the comment in7da9b01. Is that inline with what you are thinking?

-> Fix "redirect link" handling#33440

Improved the path handling logic and added some tests inlinks_test.go