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

Merged
merged 2 commits into from
Jul 12, 2017
Merged

Fix SHA1 hash linking #2143

merged 2 commits into from
Jul 12, 2017

Conversation

rsmarples
Copy link
Contributor

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.
@lafriks
Copy link
Member

lafriks commented Jul 11, 2017

Can you add additional test cases to markdown_test.go file?

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 11, 2017
@lunny lunny added this to the 1.2.0 milestone Jul 12, 2017
@lunny lunny added the type/bug label Jul 12, 2017
@lunny
Copy link
Member

lunny commented Jul 12, 2017

LGTM

@tboerger tboerger 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 Jul 12, 2017
@bkcsoft
Copy link
Member

bkcsoft commented Jul 12, 2017

LGTM

@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 Jul 12, 2017
@bkcsoft bkcsoft merged commit 89845f6 into go-gitea:master Jul 12, 2017
@bkcsoft
Copy link
Member

bkcsoft commented Jul 12, 2017

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.

Please make a PR or an Issue 😉

@lunny
Copy link
Member

lunny commented Aug 10, 2017

Anyone could back port this to release/v1.1?

lunny pushed a commit to lunny/gitea that referenced this pull request Aug 11, 2017
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 backport/done All backports for this PR have been created label Aug 11, 2017
lunny added a commit that referenced this pull request Aug 12, 2017
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.
@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
backport/done All backports for this PR have been created 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.

Commit hashes not parsed / linked in issue comments
5 participants