EnixCoda/Gitako

Branches with forward-slashes in the branch name break Gitako during navigation

tallpants opened this issue · 10 comments

Steps to reproduce:

  1. There is a branch called 6737-trailing-space/auth-fix in this repo: https://github.com/aws-amplify/amplify-js
  2. On first load, the branch is set correctly:

Screenshot 2022-06-29 at 1 00 35 PM

  1. If you navigate to a file by clicking on it in Gitako, the branch name is set incorrectly and it breaks:

Screenshot 2022-06-29 at 1 01 53 PM

  1. Reloading the page fixes it.

Here's the URL to try yourself: https://github.com/aws-amplify/amplify-js/tree/6737-trailing-space/auth-fix

Thank you @tallpants

Resolving slashes in branch name has been a long-existing pain for Gitako.

This bug is caused because return value of document.querySelector document.body.querySelector are different. The later one works properly, while document.querySelector returns null.

Very interesting...

Fixed in v3.7.4

Resolving slashes in branch name has been a long-existing pain for Gitako.

You can look at how Refined GitHub handle it:

https://github.com/refined-github/refined-github/blob/c05bc22207773ffe611ef0a0e6d2e0db45576c3a/source/github-helpers/github-url.ts#L74-L85

BTW, if you have any GitHub related issues you may have luck digging Refined GitHub codebase, or even tag me 😉.

@kidonng Thanks!

@kidonng It seems that Refined GitHub depends on encoded /, i.e. %2F, to resolve branch names with slashes from URL properly.

But general URLs do not include %2F. 🤔 Perhaps Refined GitHub also depends on DOM elements to achieve that? I thought Refined GitHub has some magic way to figure that out from URL.


Looks like it depends on document's title. I also made attempts on the title, but that did not work as well as expected. Maybe I should give another try.

Thanks anyway!

I think you had some confusion. This issue is about slashes in branch name (which is common), and #258 is about duplicate slashes (which is mostly artificial and uncommon).

Also notice that I shared different code snippets depending on the issue.

@kidonng Please take it as a late reply to your previous comment. I did not mix the 2 issues.

Also notice that I shared different code snippets depending on the issue.

I see that. And by this comment, I mean I did dig Refined GitHub codebase.

No hard feelings! I may have misunderstood your comment because the difference does seem clear to me 😥

Now that I'm taking a closer look, the code I'm referencing is a little bit off, it should be this disambiguateReference function which tells the branch and file path apart, which in turn depends on getCurrentCommittish.

Perhaps Refined GitHub also depends on DOM elements to achieve that?

Indeed. I see you have figured it out by yourself.

@kidonng Not at all, I am always grateful about your help. 😄