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 PR commit links on submission (GitHub’s bug) #2327

Closed
fregante opened this issue Aug 13, 2019 · 15 comments · Fixed by #3085
Closed

Fix PR commit links on submission (GitHub’s bug) #2327

fregante opened this issue Aug 13, 2019 · 15 comments · Fixed by #3085
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted Please! ♥︎ Particularly useful features that everyone would love!

Comments

@fregante
Copy link
Member

fregante commented Aug 13, 2019

Issuehunt badges

GitHub has had this really annoying bug that changes PR commit links (e.g. this one) into non-PR-related links like this: cb44a4e

This only happens to auto-linked URLS like https://...etc and not to [text](https://...etc)

We can't fix these links after they've been submitted but we can do it before submission.


This feature should should:

  1. Listen to comment form submissions
  2. Detect plain URLs to PR commits (and skip markdown links)
  3. Replace plain urls with identical markdown links, like [https://...etc](https://...etc)

Step 2 is easy to implement in most places (e.g. find a URL surrounded by spaces) but it can be tripped with content like [this url https://... is the problem](https://...) (which... probably does not happen)

Test comment content:

https://github.com/sindresorhus/refined-github/pull/3/commits/cb44a4eb8cd5c66def3dc26dca0f386645fa29bb

IssueHunt Summary

max-arias max-arias has been rewarded.

Backers (Total: $18.81)

Submitted pull Requests


Tips

@fregante
Copy link
Member Author

cc @lukehefson just in case he can fix this internally.

@lukehefson
Copy link

Thanks for the ping @fregante! Did you submit a bug report to our Support team? And if so, do you think you could DM (twitter, email) me the ticket reference?

@fregante fregante added the Please! ♥︎ Particularly useful features that everyone would love! label Jan 7, 2020
@issuehunt-oss
Copy link

issuehunt-oss bot commented Jan 22, 2020

@fregante has funded $18.81 to this issue.


@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label Jan 22, 2020
@max-arias
Copy link
Contributor

looks like this was solved?

@fregante
Copy link
Member Author

fregante commented May 8, 2020

cb44a4e

@fregante
Copy link
Member Author

fregante commented May 8, 2020

Nope. I just posted 1 but the link points to 2

1. https://github.com/sindresorhus/refined-github/pull/3/commits/cb44a4eb8cd5c66def3dc26dca0f386645fa29bb
2. https://github.com/sindresorhus/refined-github/commit/cb44a4eb8cd5c66def3dc26dca0f386645fa29bb

This issue is considered solved (by GitHub) when the link I just posted points to a PR commit, not a plain commit.

@max-arias
Copy link
Contributor

I'll give this a shot!

@max-arias
Copy link
Contributor

Pr up: #3085

@issuehunt-oss
Copy link

issuehunt-oss bot commented May 21, 2020

@sindresorhus has rewarded $16.92 to @max-arias. See it on IssueHunt

  • 💰 Total deposit: $18.81
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $1.89

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels May 21, 2020
@wearhere
Copy link

This is really cool—the find of feature that makes me proud to use Refined GitHub. Nice work everyone. The styling of the warning is especially nice.

@fregante
Copy link
Member Author

Glad you like it, this bug annoyed me for a long time.

Some fixes to this feature are coming in #3223

@mig4
Copy link

mig4 commented Apr 7, 2021

This doesn't really explain why GitHub changing the link is a problem. I.e. what is wrong with the link they transform the original into?

@Maxim-Mazurok
Copy link

Great feature! Minor inconsistency with GitHub:

When publishing a link in another repo, it should be like this:

Maxim-Mazurok/google-api-typings-generator@429318c

and not like this:

429318c

@fregante
Copy link
Member Author

This doesn't really explain why GitHub changing the link is a problem. I.e. what is wrong with the link they transform the original into?

The difference is subtle, but you can open the 2 links in the first paragraph of this PR and compare them.

What the user pastes:
// https://github.com/sindresorhus/refined-github/pull/3/commits/cb44a4eb8cd5c66def3dc26dca0f386645fa29bb

What GitHub displays:
// https://github.com/sindresorhus/refined-github/commit/cb44a4eb8cd5c66def3dc26dca0f386645fa29bb

In short: The correct link shows the commit on the PR page, while GitHub drops this piece of information, showing a link to the plain commit, without any connection to the PR.

@Maxim-Mazurok I opened an issue about that, thanks!

@mig4
Copy link

mig4 commented Apr 12, 2021

So if you want to link to commit, it's fine to go with GitHub's version and if you want to show the commit in context of the PR then use the button to fix the link. Makes sense.

I wondered if they do that because the link in PR context can become stale, like if the commit is rebased, but I guess the direct commit link could also then become stale after GC.

Anyway, thanks for the feature and the explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted Please! ♥︎ Particularly useful features that everyone would love!
Development

Successfully merging a pull request may close this issue.

6 participants