Added regex matching github.company.com

[MitchDizzle]

Updated github-dark to match my company's enterprise domain which looks like "github.company.com"

I'm not sure how to provide evidence that this works other than my word, I found this style and when I installed it initially did not work for my company's external domain, after this little bit of regex addition it started to work on both normal GitHub and GitHub Enterprise.

Sorry If I didn't conform to any of the contributing steps, it seems they may have been more directed at creating new styles or something?

[the-j0k3r]

@MitchDizzle thank you for reaching out to us with this pr. =)

@silverwind or @Mottie will be better people to check the merits of this enhancement, atm though enterprise users are required to add their domain to GHD, which has to be done after each style update.

I think it would be nice to have this just work out-of-the-box, though Im ot sure if thers a particular reason why it is like it is atm.

Thanks again, and lest wait for some feedback/action.

[MitchDizzle]

I think there is a few things to be concerned about also, like I noticed GitHub Enterprise instances could possibly differ on versions of the public github domain.

[the-j0k3r]

like I noticed GitHub Enterprise instances could possibly differ on versions of the public github domain.

I think if theres any other issues, they need to be properly elaborated and documented, I dont think any of us (I dont anyway) have an Enterprise account to go and test/fix or anything of the kind.

[silverwind]

Does Stylus allow user-defined include/exclude without breaking the auto-update? Would prefer the user to define their domain name instead of matching github.* which I'm sure will break other sites not related to GitHub Enterprise.

[MitchDizzle]

which I'm sure will break other sites not related to GitHub Enterprise.

Fair point, I don't know too much about stylus or how the style overriding works, but if there is an easier solution that translates to potentially all the stylus styles then I'm all for it.
However as it stands I had to do this change manually for my company's enterprise domain. If there was a way to determine if it was a github page before applying the style without the need of a regex of the domain that'd be great.

[xt0rted]

Does Stylus allow user-defined include/exclude without breaking the auto-update?

Not that I'm aware of. When I add an extra domain to the GHD style and then try to update it prompts me about my local changes being over written.

Are we able to add a configurable value to the domain list and just default it to github.com? Then anyone using GHE can add their domain(s) as needed.

[xt0rted]

Looks like you can add a configurable value to the domain list. At least in my quick test it just worked.

[MitchDizzle]

Looks like you can add a configurable value to the domain list. At least in my quick test it just worked.

Mind showing where? I looked through the Stylus extension and couldn't figure out how to add domains unless editing the .css manually

edit: oh i see having it as an option to and adding the option to the domain regex?

[xt0rted]

So this does work after all. I needed to fully close out of Stylus for it to take effect. Here's my change to prove it worked:

  } EOT;
}
+@advanced text ghe-domain "GitHub Enterprise Domain" "github.com"
==/UserStyle== */
@-moz-document
 regexp("^https?://((gist|guides|help|launch-editor|raw|resources|status|developer)\\.)?github\\.com/((?!generated_pages/preview).)*$"),
 domain("githubusercontent.com"),
 domain("graphql-explorer.githubapp.com"),
+domain("/*[[ghe-domain]]*/") {
/*! Github Dark v1.21.46 (2019-04-29) */

[the-j0k3r]

Why is google showing GHD active?

[MitchDizzle]

Why is google showing GHD active?

Because he used google.com in his test since he doesn't have access to an enterprise domain. (See image above it)

So it requires a full restart of the browser to have it take effect? Or your addition required the restart?

And since this was proven, maybe an enhancement to allow multiple domains to be added and perhaps a possible exclusion? So I can have a different github show a different style to differ between the two:
Github enterprise = GitHub Dark (Orange Green or Blue Accent)
Github.com = GitHub Dark (Orange Accent)
Right now the only way I can tell with having the style on both sites is the icon, logo and the weird difference of full screen search bar..

edit: To point out the differences between GitHub and GitHub Enterprise (Maybe add a warning for adding in domains)

[silverwind]

@xt0rted nice find, we can use this until openstyles/stylus#708 is resolved.

[xt0rted]

@MitchDizzle I think my issue was I needed to close out of the style editor for the domain change to be picked up. Usually style changes take effect immediately, but the document rule didn't seem to.

Something else I was thinking about was in addition to publishing github-dark.user.css we publish a second called github-enterprise-dark.user.css which is an exact copy of the existing style, but will allow users to not update their GHE version if they're running an older instance, and as you mentioned you'd be able to set a different primary color for GH and GHE making it a bit easier to tell which site you're on.

[silverwind]

Not a fan of publishing a second file, it's extra maintenance burden.

[noahm]

I've been wishing for support of this kind for many many years now! I agree it's important to be able to opt out of auto-updates because even a bleeding edge GHE version is often behind live github styles for weeks or months at a time. I think there might be a better approach than publishing a second file: have a designated custom build script specifically for GHE users to build their own custom file that they can update in tandem with their internal GHE version. Ideally the build would:

• output only the main @-moz-document block as all the other domains aren't relevant
• allow the user to input their internal domain and convert that to a regex of the form https?://INTERNAL_DOMAIN/(?!pages/)((?!generated_pages/preview).)*$(we need to exclude two paths as GHE can host static sites but they don't live on a separate domain like github.io for the public site)

[silverwind]

May be something that can only be supported in the script version. I could even see a a dropdown that lets the user switch between different versions of GHD in case thei GHE installation is older.

[LunaticoCR]

Hoping to bring the PR back to life here...
I'm on GHE and using this theme, installed the usercss on Firfox using the Stylus plugin. I then modified the @-moz-document to add my domain and it works great!
The problem is I'm getting tired of having to re-apply the extra domain, I presume it gets wiped on each update or something.
So here I am, wondering how can I avoid my domain customization from been overwritten.

I tried @xt0rted 's recommendation of modifying that section of code as follows:

  } EOT;
}
+ @advanced text ghe-domain "GitHub Enterprise Domain" "github.com"
==/UserStyle== */
@-moz-document
 regexp("^https?://((gist|guides|help|launch-editor|raw|resources|status|developer)\\.)?github\\.com/((?!generated_pages/preview).)*$"),
 domain("githubusercontent.com"),
 domain("graphql-explorer.githubapp.com"),
+ domain("/*[[ghe-domain]]*/") {
/*! Github Dark v1.21.46 (2019-04-29) */

This also works, and it's even better because it's so much easier to add the new domain using the Stylus plugin. Yes, you have to restart Firefox for it to fully work.

Now, my guess is that the same thing's going to happen, my changes will get overwritten and I'll be back to square one. So, can we get this diff in?

[the-j0k3r]

@LunaticoCR Thats what I was thinking about.

I agree that solution is more ideal than current, it would survive updates, and what you experience now is updates which must be forced due to local edits take precedence, does wipe local additions out.

So here s my +1 for that solution.

The alternative is a fork + these additions that you keep rebased and update from the fork thus also needing to change usercss metadata withing the build tools. Thats always an alternative, to waiting until the eventual day it gets added, I guess.

[LunaticoCR]

Thats always an alternative, to waiting until the eventual day it gets added, I guess.

But if the project owners agree we can just add it now right? I'm going to assume the top contributors own it so I'll mention them here: @silverwind @Mottie @the-j0k3r @xt0rted

So we can open a new PR for this, the change would be just on the usercss. I don't mind opening the PR myself if you prefer? Although the rightful author is @xt0rted , but I can always mention you and give you all the credit.

Let me know what you guys think.

[the-j0k3r]

To me I would merge such PR immediately,, thought its not just up to me =)

Add a new PR that also included on the issue description closes #938 and away to the races.

[auscompgeek]

I think even that is overengineered. A simple url-prefix(https://github.), url-prefix(https://gist.github.) would suffice, no? (This of course assumes a GitHub Enterprise instance is deployed to a github subdomain, which is probably the case for most installations.)

Note that @LunaticoCR's suggestion won't work as expected - the regex is clearly intended to not match the entire github.com domain.

[LunaticoCR]

@auscompgeek

(This of course assumes a GitHub Enterprise instance is deployed to a github subdomain, which is probably the case for most installations.)

That's the wrong assumption, the GHE I work on is not on a github subdomain. I bet most GHE installations for big companies are on their own domains.

the regex is clearly intended to not match the entire github.com domain.

What regex? It's just taking a user provided string from the stylus plugin and using that as is, as an allowed domain. Unless I'm missing something?

[auscompgeek]

That's the wrong assumption, the GHE I work on is not on a github subdomain. I bet most GHE installations for big companies are on their own domains.

Counterpoints: the OP's GitHub Enterprise is on a github subdomain, and my university's GHE is at github.sydney.edu.au as well. I don't know what you mean by "on their own domains", that's a requirement of self-hosting in the first place.

What regex?

The existing document regex.

[the-j0k3r]

I bet most GHE installations for big companies are on their own domains.

I wouldnt know, but to be sure one would have to know actual examples and what formats they use and if the proposed solution accommodates the most actual cases.

[LunaticoCR]

Thanks for the explanation @xt0rted , I see your point.
I'm not too sure that all GHE installations will have that kind of setup, in my case for example the gists are at https://xyz-github.company.zyx/gist, and the pages feature is not enabled. Now, my GHE setup could be unique, or it could be the norm, I don't know.

[the-j0k3r]

I would say if any of the given custom solutions solve the most cases reported here, go with that. Its a start.

[auscompgeek]

I'm not too sure that all GHE installations will have that kind of setup

Yes, this is true, there are two supported configurations each for Gist and Pages in GHE. I would personally wager that there are more GHE installs with Gist and Pages on subdomains, but there's no way to actually get stats on that.

Given that url-prefix is indeed also not correct, it looks like the only way for this to work reliably for everyone is to make it possible to add your custom domain to the regex. I suppose that might be possible, but it'll probably be messy.

[the-j0k3r]

See #973 feature request for a format that would need to be accounted here, I think.

[LunaticoCR]

it looks like the only way for this to work reliably for everyone is to make it possible to add your custom domain to the regex. I suppose that might be possible, but it'll probably be messy.

Thinking about this.... does anyone know if it's possible to insert a variable in the middle of a regex?

[Mottie]

does anyone know if it's possible to insert a variable in the middle of a regex?

It won't work. Stylus doesn't work exactly as expected, but I opened an issue where we could discuss this problem.

[LunaticoCR]

That's great, thanks for opening that issue @Mottie !

[arizaju12]

Hit_****_

[the-j0k3r]

@arizaju12 read #938 (comment)

[silverwind]

This PR seems like the wrong approach but I could see us adding a domain("/*[[custom-domain]]*/") with default value set to some invalid domain name.

[silverwind]

Closing this as it's not the right approach. Follow the issue #1086 instead.