Skip to content

Move page detection checks to a separate file#123

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

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

Conversation

@hkdobrev
Copy link
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

const ownerName = path.split('/')[1];
const repoName = path.split('/')[2];
const segments = location.pathname.split('/');
const ownerName = segments[1];
Copy link
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
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
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
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
Copy link
Contributor

DrewML commented Mar 31, 2016

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

-->
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