Skip to content

Conversation

@mrsdizzie
Copy link
Member

Ugh sorry this is the same as #6153 which I broke by deleting my branch (which github didn't like)

Anyway, this is the same request to modify linkRegex so it only matches text starting with http|https when creating links. I've also added the requested tests for this to the current test suite as well.

Modify the current linkRegex to require http|https which appears to be
the intended behavior based on the comments. Right now, it also matches
anything starting with www as well. Also add testing for linkRegex
@codecov-io
Copy link

codecov-io commented Feb 23, 2019

Codecov Report

Merging #6171 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6171      +/-   ##
==========================================
- Coverage   38.86%   38.85%   -0.01%     
==========================================
  Files         354      354              
  Lines       50210    50210              
==========================================
- Hits        19514    19510       -4     
- Misses      27869    27874       +5     
+ Partials     2827     2826       -1
Impacted Files Coverage Δ
modules/markup/html.go 90.07% <ø> (+0.47%) ⬆️
models/unit.go 0% <0%> (-14.29%) ⬇️
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 4b7237b...aa51910. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 23, 2019
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

I'm going to approve, as leaving the broken autolinker for www.* in is of no use whatsoever.

@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 Feb 26, 2019
@lafriks lafriks added the type/enhancement An improvement of existing functionality label Feb 27, 2019
@lunny
Copy link
Member

lunny commented Feb 28, 2019

Is this a break change?

@zeripath
Copy link
Contributor

Well the current situation is that the auto links to say www.google.com just don't work and have never worked.

@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 Feb 28, 2019
@lunny lunny added this to the 1.8.0 milestone Feb 28, 2019
@lunny
Copy link
Member

lunny commented Feb 28, 2019

So this should be back port to release/v1.7

@lunny lunny merged commit 4a2e92b into go-gitea:master Feb 28, 2019
@lunny
Copy link
Member

lunny commented Feb 28, 2019

Please send back port to release/v1.7

@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/bug type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants