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

Fix SHA1 hash linking (#2143) #2293

Merged
merged 1 commit into from
Aug 12, 2017
Merged

Conversation

lunny
Copy link
Member

@lunny lunny commented Aug 11, 2017

Back port from #2143.

This changes the regex to look for a hash from 7 to 40 characters,
to match the use of abbreviated hash lookups in both git and github.
The restriction of not being a pure number is also removed because
1234567 is now considered a valid abbreviated hash, as is deadbeef.

A note has been added to the top of the code to state that the
literal regex match is fine, but no extra validation is currently
performed so some false positives are expected.

A future change could ensure that the hash exists in the repository
before rendering it as a link, although this might incur a slight
performance penalty.

Reverts part of commit 4a46613 and fixes #2053.

This changes the regex to look for a hash from 7 to 40 characters,
to match the use of abbreviated hash lookups in both git and github.
The restriction of not being a pure number is also removed because
1234567 is now considered a valid abbreviated hash, as is deadbeef.

A note has been added to the top of the code to state that the
literal regex match is fine, but no extra validation is currently
performed so some false positives are expected.

A future change could ensure that the hash exists in the repository
before rendering it as a link, although this might incur a slight
performance penalty.

Reverts part of commit 4a46613 and fixes go-gitea#2053.
@lunny lunny added the type/bug label Aug 11, 2017
@lunny lunny added this to the 1.1.4 milestone Aug 11, 2017
@sapk
Copy link
Member

sapk commented Aug 11, 2017

LGTM

@tboerger tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Aug 11, 2017
@sapk
Copy link
Member

sapk commented Aug 11, 2017

@lunny you should add the tag backport/v1.1 to this PR

@lunny
Copy link
Member Author

lunny commented Aug 11, 2017

@sapk No. The tag have been added to #2143. This PR's target is go-gitea:release/v1.1. There is no other label to tell us. But for #2143, we should know if that PR has been backported to which branch.

@sapk
Copy link
Member

sapk commented Aug 11, 2017

ok sorry, I misunderstood the tag.

@ethantkoenig
Copy link
Member

LGTM

@ethantkoenig
Copy link
Member

make L-G-T-M work

@tboerger tboerger 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 Aug 12, 2017
@lunny lunny merged commit 1709297 into go-gitea:release/v1.1 Aug 12, 2017
@lunny lunny deleted the lunny/backport_2143 branch August 12, 2017 01:33
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants