Don't manually replace whitespace during render

[mrsdizzie]

For historical reasons Gitea manually alters the urlPrefix and replaces a whitespace with a +. This Works for URLs, but we're also passing urlPrefix to git calls and adding the + is breaking the tree path.

Goldmark will automatically convert a white space to the proper %20, so we should leave the string as is which lets us pass it to git unmodified and then let Goldmark fix any whitespaces when rendering. This keeps treePath and the generated link working. See #10156 for more details.

Fixes #10156

Here is example of broken + behavior that is fixed:

https://try.gitea.io/mrsdizzie/testcase/src/branch/master/test%20a

[mrsdizzie]

Hmm will need more though on escaping elems here

[mrsdizzie]

@guillep2k had to revert escaping the elements because it breaks valid fragment links like mypage#section1 and would involve more logic rewriting in general. I notice a few other bugs with filenames that have a pound sign/how we handle what we think are anchors in markdown and I think all of that logic needs to be looked into in a separate PR.

This PR will still fix issues mentioned above with regards to folder names and filenames with white spaces with just a few lines.

[guillep2k]

This PR will still fix issues mentioned above with regards to folder names and filenames with white spaces with just a few lines.

Fair enough. Please send an empty commit to restart CI.

[guillep2k]

I've been reading the code, and now I'm not so sure about URLJoin escaping the base parameter. The function is used everywhere, and some places might have the base already escaped (e.g. what happens with utf-8 domain names? Or a Gitea URL that has a relative path?). Perhaps it's a better idea to have a GitPathToHttpUrl() function, and use it when necessary, leaving URLJoin to do only one job.

[mrsdizzie]

I've been reading the code, and now I'm not so sure about URLJoin escaping the base parameter. The function is used everywhere, and some places might have the base already escaped (e.g. what happens with utf-8 domain names? Or a Gitea URL that has a relative path?). Perhaps it's a better idea to have a GitPathToHttpUrl() function, and it when necessary, leaving URLJoin to do only one job.

You're right, anything that needs to should be escaped before being passed to that function or needs more thought anyways.

I've just remove that change from this PR as it should be part of another refactor that addresses #10300 better (which is how I came across it). Updated PR description as well.

[codecov-io]

Codecov Report

Merging #10291 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #10291      +/-   ##
==========================================
+ Coverage   43.68%   43.69%   +0.01%     
==========================================
  Files         585      585              
  Lines       81010    81015       +5     
==========================================
+ Hits        35390    35402      +12     
+ Misses      41238    41226      -12     
- Partials     4382     4387       +5

Impacted Files	Coverage Δ
modules/markup/markup.go	76.81% <ø> (-0.34%)	⬇️
modules/markup/markdown/goldmark.go	66.07% <ø> (-0.31%)	⬇️
modules/indexer/stats/queue.go	62.5% <0%> (-18.75%)	⬇️
modules/indexer/stats/db.go	40.62% <0%> (-18.75%)	⬇️
modules/git/utils.go	65.67% <0%> (-4.48%)	⬇️
modules/notification/ui/ui.go	70.37% <0%> (-4.1%)	⬇️
services/pull/temp_repo.go	29.05% <0%> (-2.57%)	⬇️
services/pull/patch.go	60.37% <0%> (-2.52%)	⬇️
models/unit.go	37.03% <0%> (-2.47%)	⬇️
modules/git/repo.go	46.78% <0%> (-0.92%)	⬇️
... and 7 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15614a8...80e8cba. Read the comment docs.