Fix broken patch/diff links on commit PRs. Fixes #104#112
Fix broken patch/diff links on commit PRs. Fixes #104#112sindresorhus merged 1 commit intorefined-github:masterfrom
Conversation
extension/content.js
Outdated
| let commitUrl = location.href.replace(/\/$/, ''); | ||
| const prCommit = /\/pull\/\d+\/commits/; | ||
| if (prCommit.test(commitUrl)) { | ||
| commitUrl = commitUrl.replace(prCommit, '/commit'); |
There was a problem hiding this comment.
This check does not handle forks well. I'm not sure if it's easy to support forks in this scenario though.
There was a problem hiding this comment.
I figured it wouldn't be this easy :) Do you have an example link to a page where it breaks down?
There was a problem hiding this comment.
Hmm, actually when checking it out, it seems GitHub copies the commits in the base repo so it's working properly even with PRs from forks.
|
LGTM 💫 |
#99 landed. Can you do this? |
|
No problem - will push an update after work tonight. |
|
On second thought, I'm going to sit on this until #123 goes in (it's pretty small), and then I'll make the update to consume the new page-detection module. |
Hehe, I thought I'll update this code in #123 after this is merged as this is a bugfix and #123 is changing more things and needs to be more thoroughly tested. |
|
@hkdobrev Cool, that works for me. Wasn't going to throw the work over the fence to you, but now I will :) |
|
Update pushed. |
|
LGTM |
3 participants
Note: Once #99 goes in, it would make sense to switch out the
prCommitregex local to this function with theisPRCommitregex (the one that will be moving to the URL utils file shortly).