Skip to content

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Apr 5, 2019

This improves the SHA1 link detection to not pick up extraneous non-whitespace characters at the end of the URL. The '.' is a special case handled in code itself because of missing regexp lookahead support.

Regex test cases: https://regex101.com/r/xUMlqh/3/

Before

Screenshot 2019-04-06 at 01 09 57

After

Screenshot 2019-04-06 at 01 10 04

This improves the SHA1 link detection to not pick up extraneous
non-whitespace characters at the end of the URL. The '.' is a special
case handled in code itself because of missing regexp lookahead
support.

Regex test cases: https://regex101.com/r/xUMlqh/3
@codecov-io
Copy link

codecov-io commented Apr 5, 2019

Codecov Report

Merging #6526 into master will decrease coverage by 0.02%.
The diff coverage is 69.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6526      +/-   ##
==========================================
- Coverage   40.28%   40.26%   -0.03%     
==========================================
  Files         403      403              
  Lines       54069    54079      +10     
==========================================
- Hits        21784    21775       -9     
- Misses      29268    29286      +18     
- Partials     3017     3018       +1
Impacted Files Coverage Δ
modules/markup/html.go 81.93% <69.23%> (-1.37%) ⬇️
modules/avatar/avatar.go 81.25% <0%> (-18.75%) ⬇️
models/unit.go 0% <0%> (-14.29%) ⬇️
models/repo_list.go 66.84% <0%> (-1.06%) ⬇️
routers/repo/view.go 41.08% <0%> (-1%) ⬇️

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 0bdd81d...3e400a2. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 5, 2019
@lunny lunny added the type/enhancement An improvement of existing functionality label Apr 6, 2019
@silverwind
Copy link
Member Author

Just in case anyone is wondering: My motiviation for this change is that I like to post fixed in <paste link to commit>. to close an issue.

The dot at the end caused the link to become invalid because the old parser appended /. to it, thinking it is a path. The new mechanism treats the dot as not part of the URL, like GitHub also does.

@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 Apr 6, 2019
@lafriks lafriks added this to the 1.9.0 milestone Apr 6, 2019
@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 Apr 6, 2019
@lafriks lafriks merged commit 2242a9f into go-gitea:master Apr 6, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants