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

Add isCompareWikiPage #118

Merged
merged 4 commits into from
Jun 29, 2022
Merged

Add isCompareWikiPage #118

merged 4 commits into from
Jun 29, 2022

Conversation

kidonng
Copy link
Member

@kidonng kidonng commented Jun 1, 2022

Despite being called "Compare", this page type is more like a crossover between isCommit and isGistRevision, which means easy-toggle-files can support it when this get merged.

index.ts Outdated
@@ -54,6 +54,12 @@ collect.set('isCompare', [
'https://github.com/sindresorhus/refined-github/compare/test-branch?quick_pull=1',
]);

export const isCompareWikiPage = (url: URL | HTMLAnchorElement | Location = location): boolean => isRepoWiki(url) && url.pathname.split('/').at(-2) === '_compare';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call me lazy, this should be using regex. Though as of now Array.at() should only bite Safari < 15.4, which may be a lot.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use a negative index at all? Isn't it safer counting from the start? That's how all URLs here work

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, somehow when I discovered the page name can be ommited, I thought there can be infinite level of pages, but in fact it is either zero or one.

@fregante
Copy link
Member

fregante commented Jun 1, 2022

You can use .slice(3,2).includes(_compare). But now that I'm looking at the URLs maybe the negative index makes sense. Slice also works with negative indexes so we can keep supporting Safari. I'm not totally sure it's needed, but…

@kidonng
Copy link
Member Author

kidonng commented Jun 1, 2022

As described above negative indexing is not mandatory. Slice is fine.

The biggest consumer of this package is RGH, which has quite a lot Safari users, so use something that requires the latest version of Safari isn't appealing. This is the reason why I brought it up first.

@kidonng kidonng merged commit 821502a into main Jun 29, 2022
@kidonng kidonng deleted the isCompareWikiPage branch June 29, 2022 14:37
@kidonng
Copy link
Member Author

kidonng commented Jun 29, 2022

I ran the publish workflow after merging this PR but got no response. Now when I check it seems to have delayed ~15 minutes #itsamystery

@fregante
Copy link
Member

I think Actions is/was down. I didn't see the CI on refined-github/refined-github#5748 for a while too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants