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

Commit hashes not parsed / linked in issue comments #2053

Closed
2 of 7 tasks
IzzySoft opened this issue Jun 24, 2017 · 6 comments · Fixed by #2143
Closed
2 of 7 tasks

Commit hashes not parsed / linked in issue comments #2053

IzzySoft opened this issue Jun 24, 2017 · 6 comments · Fixed by #2143
Labels
Milestone

Comments

@IzzySoft
Copy link

  • Gitea version (or commit ref): 1.1.2
  • Git version: 2.1.4
  • Operating system: Bananian (Debian Jessie, ARM)
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

When referencing commit hashes within issue comments, they are never linked to the corresponding commit – whether separate on a line or enclosed by text (yes, I saw #1667), whether shortened to 7, 10 or not at all (in which case the parser shortens the string, but doesn't link it) – no link.

Of course the corresponding commits exist in the very same project, so it's not that there'd be no target. As the linking does work on try.gitea.io it seems that either something must be missing with my installation (single-binary drop-in: gitea-1.1.2-linux-arm-7), or this is a bug specific to the ARM build.

Example

For this input:

foo

5c5c946673 

bar 5c5c946673ebd1fdcc996a5a198699b4e3ace9b4

Result is rendered like this:

<div class="ui bottom attached tab segment markdown active" data-tab="preview"><p>foo</p>

  <p>5c5c946673</p>

  <p>bar 5c5c94667</p>
</div>

The last line suggests Gitea did recognize it as hash, as otherwise I don't see why it got shortend.

@lunny lunny added the type/bug label Jun 25, 2017
@lunny lunny added this to the 1.x.x milestone Jun 25, 2017
rsmarples added a commit to rsmarples/gitea that referenced this issue Jul 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.
bkcsoft pushed a commit that referenced this issue Jul 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.
@lunny lunny modified the milestones: 1.2.0, 1.x.x Jul 12, 2017
@IzzySoft
Copy link
Author

Just updated to the fresh new 1.1.3 and wonder: does the milestone 1.2.0 mean the fix won't be included before that? As the issue still persists with v1.1.3. Same for #1541 (which was marked fixed even before v1.1.2 was released).

@lunny
Copy link
Member

lunny commented Aug 10, 2017

Yes, these two PR haven't be backported to v1.1.3. But maybe we can backported it to v1.1.4.

@IzzySoft
Copy link
Author

No deal-breaker, I just tried to understand. But of course, the sooner the better 😉

lunny pushed a commit to lunny/gitea that referenced this issue 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
Copy link
Member

lunny commented Aug 11, 2017

@IzzySoft #2291 and #2293 both are backported to v1.1.4

@IzzySoft
Copy link
Author

@lunny Thanks a lot!

lunny added a commit that referenced this issue 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.
@IzzySoft
Copy link
Author

Yes! Working in v1.2 – thanks again!

@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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants