Skip to content

Move page detection checks to a separate file#123

Merged
hkdobrev merged 1 commit into
masterfrom
page-detect
Mar 31, 2016
Merged

Move page detection checks to a separate file#123
hkdobrev merged 1 commit into
masterfrom
page-detect

Conversation

@hkdobrev

Copy link
Copy Markdown
Contributor

Closes #120.

In this change I've tried to improve readability and consistency and not focus so much on performance as microoptimisations in these checks are not worth it.

Except for moving the checks to a separate file I've unified them a bit and made some fixes:

  • simplified and unified dashboard regular expressions
  • made all regex checks nested under a repo consistent
  • re-used as much as possible especially for commit checks
  • simplified repo root check
  • fixed repo root check with tree in the regex for repositories like https://github.com/react-component/tree
  • tried to use consistent naming and order of the methods
  • fixed a few regular expressions not taking trailing slash in the URL into account

Comment thread extension/content.js Outdated
const ownerName = path.split('/')[1];
const repoName = path.split('/')[2];
const segments = location.pathname.split('/');
const ownerName = segments[1];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Stylistic: How about const [, ownerName, repoName] = location.pathname.split('/');?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 Destructuring became available in Chrome 49, so we can start using it now. (Need to bump the min Chrome version in the manifest.

@sindresorhus

Copy link
Copy Markdown
Member

This probably also needs an update because of #112 (comment).

Closes #120

In this change I've tried to improve readability and consistency and not
focus on performance as microoptimisations in these checks are not worth
it.

Except for moving the checks to a separate file I've unified them a bit
and made some fixes:

- simplified and unified dashboard regular expressions
- made all regex checks nested under a repo consistent
- re-used as much as possible especially for commit checks
- simplified repo root check
- fixed repo root check with `tree` in the regex for repositories like
https://github.com/react-component/tree
- tried to use consistent naming and order of the methods
- fixed a few regular expressions not taking trailing slash in the URL
into account
@hkdobrev

Copy link
Copy Markdown
Contributor Author

I think I've addressed all comments. /cc @sindresorhus @DrewML

@hkdobrev hkdobrev added the meta Related to Refined GitHub itself label Mar 31, 2016
@DrewML

DrewML commented Mar 31, 2016

Copy link
Copy Markdown
Contributor

LGTM!

@hkdobrev hkdobrev merged commit f959163 into master Mar 31, 2016
@hkdobrev hkdobrev deleted the page-detect branch March 31, 2016 22:03
busches added a commit that referenced this pull request Mar 15, 2019
This changes the 'Diff settings' text to the settings icon and removes
the whitespace options if `diff-view-without-whitespace-options` is
enabled.

Example URL:
https://github.com/sindresorhus/refined-github/pull/1146/files

Closes #1292

![image](https://user-images.githubusercontent.com/857700/52908034-dc763a00-3232-11e9-8b8c-faa77c373b2a.png)


<!--

The more the merrier! 🍻 Thanks for contributing!

1. List some URLs that reviewers can use to test your PR

2. Make sure you specify the issue you're fixing/closing, if any, following this format:

Fixes #123
Closes #56

-->
Copilot AI added a commit that referenced this pull request May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meta Related to Refined GitHub itself

Development

Successfully merging this pull request may close these issues.

Extract all page detection checks to a new file

3 participants