Repair GH repo page url view only to point to master as committed originally...

[Martii]

... in #89

Applies to #200

• I currently see no advantage of pointing the view url to the current tree/HEAD when clicking the url to go to GH during the import process. I do however see a disadvantage of not being able to traverse up their breadcrumb list and being locked into a specific commit hash
• Since the import process is probably dependent upon /tree/HEAD now leaving that alone
• Add target="_blank" to this url since we do this anyways with multiple entries and keeps that window/tab open for importing after checking with GH
• Fix white space issue with optional target after POST routine returns

---

Tested in dev okay.

[Martii]

This still leaves the encoding issue mentioned in #200 (comment) with users that may use %20, or the like, in their filename/filepath not being able to view the repo subfolder.

EDIT:

This presents an issue with being able to do this without breaking import. If I encode full, dir, name, ext and filename it will break things.

1. I could create another variable set with suffix of Enc or Encoded e.g. fullEncoded for use in mustache with viewing the page.... or
2. another object named pathAsEncoded, or the like, to reuse the property names directly.

Please let me know if things should be different but I'm leaning towards option 2 (could be swayed) unless we already have something in place? (maybe waterfall, or perhaps this is overkill or incorrect usage thought ??... btw when encodeURI and encodeURIComponent is applied the variable names should probably be ...FromUtf8, ...AsEnc, ...AsEncoded, or ...AsASCII not ...AsUtf8 because the current anonymous functions in waterfall are taking UTF-8 and transforming to plain percent encoded ASCII) ...

[sizzlemctwizzle]

I'm going to defer to @Zren's opinion on this PR since he wrote the new GH integration code.

[Zren]

Doesn't encodeURIComponent(javascriptBlob.path.filename) escape \? Think you mentioned something like that elsewhere.

encodeURIComponent('OpenUserJs/OpenUserJS.org/pull/204')
"OpenUserJs%2FOpenUserJS.org%2Fpull%2F204"
encodeURI('OpenUserJs/OpenUserJS.org/pull/204')
"OpenUserJs/OpenUserJS.org/pull/204"

---

I used tree/HEAD as that's what the old code used. You can also set a different "main" branch. However, nobody does that, so it's probably better to use master anyways to be user friendly, as HEAD redirects to the HEAD commit sha.

+1 for that commit.

---

Fix white space issue with optional target after POST routine returns

Eh? Does the trailing whitespace in <a href="asdf" > break something/raise a warning? It's much easier to read the as the "opening" bit {#asdf} is together with it's children + closing "tag". Word wrap will keep it all together as well.

[Zren]

You'll probably also want to change this line: https://github.com/OpenUserJs/OpenUserJS.org/blob/master/libs/githubClient.js#L82 to "master" (i think that should work) to be consistent

[Martii]

You can also set a different "main" branch.

Very Good point I'll back that out... no need to mess that up... forgot about that since I never use that GH feature.

break something/raise a warning?

Validation for eventual HTML5 and SEO.

It's much easier to read the as the "opening" bit

Moot point because if multiple ones are added in at any point it just makes it "harder to read" before or after... so no difference.

Doesn't encodeURIComponent(javascriptBlob.path.filename) escape \?

Yes as you pointed out in the code sample... I can change the affected ones to encodeURI. But the real question is does github allow a filename with any reserved characters? e.g. ; , / ? : @ & = + $ NOTE: the backslash is Windows specific and usually one can't create a filename/filepath with that in it.

[Zren]

But the real question is does github allow a filename with any reserved characters?

Nope.

TestUserScript / folder / test@.js

https://github.com/ZrenTest/TestUserScript/blob/master/folder/test%40.js

---

Edit:

https://github.com/ZrenTest/TestUserScript/blob/master/folder/test@.js will automatically redirect, so encodeURI is okay.

---

EditEdit: Nvm. The redirect was from HEAD => sha. GitHub doesn't redirect/sanitize the path when it sees special characters, nor does it break.

[Martii]

Nope... Nvm.

So which would you recommend here and on which ones?

so it's probably better to use master anyways to be user friendly, as HEAD redirects to the HEAD commit sha.

For your addition... Annoyance versus breakage is the thing here... I'd rather see a semi-grouchy author having to traversing the tree from top of the repo or having to type in /blob/master than have an author say it's pulling the wrong HEAD in. You found the advantage that I didn't see right off the bat... nice catch btw.

[Zren]

Seems like there's a githubRepo.default_branch variable we could use.
https://developer.github.com/v3/repos/#get

And use encode URI

[Martii]

Seems like there's a githubRepo.default_branch variable we could use.

That could work too but I'm still a little green on the actual import process here and not sure where to read that in yet... and if needed to be applied to waterfall here.

[Zren]

<a href="https://github.com/{{repo.owner.login}}/{{repo.name}}/tree/HEAD/{{pathAsEncoded.full}}" target="_blank">{{path.dir}}<b>{{path.name}}</b>{{path.ext}}</a>
<a href="https://github.com/{{repo.owner.login}}/{{repo.name}}/tree/{{repo.default_branc}}}/{{pathAsEncoded.full}}" target="_blank">{{path.dir}}<b>{{path.name}}</b>{{path.ext}}</a>

Maybe? We don't need to change head to repo.default_branch in our api consumer code as they should be equivalent.

[Martii]

they should be equivalent.

Appears to be and it redirects here to blob/master :) Nice! Does that need to be encodeURId too somewhere? ;)

[Martii]

Does that need to be encodeURId too somewhere?

Talking to myself out loud here... Yes it does... and I don't have a clue which source to do this in... :\ There are too many repo.name and name in other objects floating around in various parts to track down which one is actually used here... guess I'll do blind stabbing and testing.

[Martii]

Alright... how is this @Zren @sizzlemctwizzle

That commit appears to be at the right spot... fixes the default repo HEAD naming issue is there is a branch that has a percent symbol (or other) like on my test repo... this won't stay for long so go visit soon. ;)

Thought of another instance where plain blob/master could cause some headaches ... when an owner deletes it after pointing to another branch... so the tree/ + whatever we pull in from the default branch is well suited for this. :)