Opening a gist repository gives a fetch error

[burkeholland]

Fixes #2324

[mcornella]

I think the fix would be either to add gist.github.com = false to the _servers Map, or to look at host.authority to see if it's gist.github.com. This is a sample value of host on a gist repo:

{
	"authority": "gist.github.com",
	"fragment": "",
	"path": "/20dec7376005dec0a1...",
	"query": "",
	"scheme": "https"
}

[mcornella]

This works for me for both git@github.com and git@gist.github.com, while keeping functionality for non-gist repos:

diff --git a/src/authentication/githubServer.ts b/src/authentication/githubServer.ts
index a3859d4..dc87dc0 100644
--- a/src/authentication/githubServer.ts
+++ b/src/authentication/githubServer.ts
@@ -17,6 +17,11 @@ export class GitHubManager {
 			return false;
 		}
 
+		// gist repos are not supported
+		if (host.authority === 'gist.github.com' || /^\/[a-z0-9]+$/i.test(host.path)) {
+			return false;
+		}
+
 		if (this._servers.has(host.authority)) {
 			return !!this._servers.get(host.authority);
 		}

EDIT: summary of what it works on:

1. git@gist.github.com:gist-id
2. git@github.com:gist-id
3. https://gist.github.com/gist-id
4. https://gist.github.com/user/gist-id

I'm unaware if https://github.com/id is a valid normal repository remote URL, which this patch would break. If it is, removing the regex test part of the condition would work, but then (2) wouldn't be detected as a gist.

[alexr00]

Does this need to be a global regular expression match (/g)?

[burkeholland]

Doesn't. What do you think of the proposed solution from @mcornella? It appears to handle the github.com case as well. I have no idea what that regex does, but his solution for gist.github.com looks more elegant than mine.

[alexr00]

I usually go here to visualize regular expressions: https://regexper.com/#%5E%5C%2F%5Ba-z0-9%5D%2B%24

I don't think we want to exclude all github hosts with non-alphanumeric characters in them (I have no idea how often those will occur with enterprise for example). I'm happy with the simple test that this PR uses.

[mcornella]

I'm ok with ignoring that part and just check the domain name. For that though we need to check host.authority instead of host.path.

[burkeholland]

Removed the g for now

[alexr00]

@burkeholland, @mcornella is correct, it is host.authority then needs to be checked for gist.github.com.

[burkeholland]

Implemented hostpath check as per @mcornella

[alexr00]

Just #2933 (comment) then we're good to merge.

[alexr00]

Doesn't host.authority need to be checked instead of host.path?

[alexr00]

Ready to merge now!