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

Modify linkRegex to require http|https #6171

Merged
merged 3 commits into from
Feb 28, 2019
Merged

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