Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply encodeURIComponent() to pieces of the custom remote URL #2336

Closed
amir-f opened this issue Nov 10, 2022 · 19 comments
Closed

Apply encodeURIComponent() to pieces of the custom remote URL #2336

amir-f opened this issue Nov 10, 2022 · 19 comments
Assignees
Labels
feature New feature or request needs-verification Request for verification
Milestone

Comments

@amir-f
Copy link

amir-f commented Nov 10, 2022

Description

We have set a custom remote by specifying the following dictionary in the .vscode/settings.json file

"gitlens.remotes": [
    {
      "domain": "git.foo.com",
      "type": "Custom",
      "name": "Foo",
      "protocol": "https",
      "urls": {
        "repository": "https://foo.com/${repo}",
        "branches": "https://foo.com/${repo}/branches/master/",
        "branch": "https://foo.com/${repo}/browse/${branch}/",
        ...
      }
    }
  ],

However the branch name ${branch} can itself have the / in it and this would make the URL invalid. What is the best way to apply encodeURIComponent() to the branch name? That is:

https://foo.com/${repo}/browse/encodeURIComponent(${branch})/

GitLens Version

13.0.4

VS Code Version

Version: 1.73.1 (Universal)
Commit: 6261075646f055b99068d3688932416f2346dd3b
Date: 2022-11-09T02:08:38.961Z (22 hrs ago)
Electron: 19.0.17
Chromium: 102.0.5005.167
Node.js: 16.14.2
V8: 10.2.154.15-electron.0
OS: Darwin arm64 21.6.0

Git Version

git version 2.37.0 (Apple Git-136)

Logs, Screenshots, Screen Captures, etc

No response

@amir-f amir-f added potential-bug triage Needs to be looked at labels Nov 10, 2022
@amir-f amir-f changed the title For custom remote services there is no way to apply encodeURIComponent() to pieces of the URL encodeURIComponent() to pieces of the custom remote URL Nov 10, 2022
@amir-f amir-f changed the title encodeURIComponent() to pieces of the custom remote URL Apply encodeURIComponent() to pieces of the custom remote URL Nov 10, 2022
@eamodio
Copy link
Member

eamodio commented Nov 12, 2022

What are you seeing when you open a branch on this remote? I've tried the provided setup with branches with / in them and it seems works to work fine.

@eamodio eamodio added needs-more-info Needs further information, steps, details, etc. and removed triage Needs to be looked at labels Nov 12, 2022
@amir-f
Copy link
Author

amir-f commented Nov 13, 2022

The slash shows up in the URL but the server expect it to be URL encoded if it's the branch name. That is if the branch name is ab/cd This URL wont' work: https://foo.com/some_repo/browse/ab/cd/ but this one does: https://foo.com/some_repo/browse/ab%2Fcd/

@eamodio
Copy link
Member

eamodio commented Nov 13, 2022

Oh, hrm. What type of Git server is it?

@amir-f
Copy link
Author

amir-f commented Nov 14, 2022

It's hosted using Phabricator

@github-actions
Copy link

This issue needs more information and has not had recent activity. Please provide the missing information or it will be closed in 7 days. Thanks!

@github-actions github-actions bot added the inactive Issue has not had recent required feedback label Nov 24, 2022
@github-actions
Copy link

github-actions bot commented Dec 2, 2022

Closing this issue because it needs more information and has not had recent activity. Please re-open this issue if more details can be provided. Thanks!

@github-actions github-actions bot closed this as completed Dec 2, 2022
@amir-f
Copy link
Author

amir-f commented Dec 5, 2022

what other information is needed?

@eamodio eamodio added triage Needs to be looked at and removed needs-more-info Needs further information, steps, details, etc. inactive Issue has not had recent required feedback labels Dec 5, 2022
@eamodio
Copy link
Member

eamodio commented Dec 5, 2022

Sorry I was travelling and the bot got to it before I labelled it appropriately.

@eamodio eamodio reopened this Dec 5, 2022
@eamodio
Copy link
Member

eamodio commented Dec 5, 2022

I don't know if there is a good way to implement this, without adding support for Phabricator itself, as I'm guessing the type of url encoding would be very specific based on the url itself. I also haven't heard this request before -- so I'm unsure if this is a Phabricator "thing" or the way you are hosting it?

@eamodio eamodio added feature New feature or request and removed potential-bug triage Needs to be looked at labels Dec 6, 2022
@eamodio eamodio added this to the Backlog milestone Dec 6, 2022
@amir-f
Copy link
Author

amir-f commented Dec 12, 2022

I see. I am not sure if it's Phabricator specific but fwiw this is an example URL pattern that works without / in the branch name:
"fileInBranch": "https://phabricator.foo.com/${repo}/browse/${branch}/${file}${line}"

so IIUC if there's a slash in ${branch} it'll trip up the parser

@micahlt
Copy link

micahlt commented May 24, 2023

This would be a great (and fairly easy-to-do) addition! The change would have to occur in https://github.com/gitkraken/vscode-gitlens/blob/fe5d12f93f825d4e8d5228bfd8fe449600877492/src/git/remotes/custom.ts

@eamodio eamodio modified the milestones: Backlog, 13.7 May 24, 2023
@eamodio eamodio self-assigned this May 24, 2023
@eamodio eamodio added pending-release Resolved but not yet released to the stable edition needs-verification Request for verification labels May 24, 2023
@eamodio
Copy link
Member

eamodio commented May 24, 2023

The main issue here is that some providers need certain encoding and others done. To handle this, I've added alternate versions of all the tokens with a _encoded suffix, e.g. ${branch_encoded} to allow for granular control over how the URL should be constructed.

NOTE, if you use any *_encoded tokens in the format string, we won't URL encode the whole URL, to give fine-grained control.

Can you please verify this fix in v2023.5.25xx pre-release edition of GitLens? Thank you so much!

You can switch to the pre-release edition of GitLens, by clicking on the "Switch to Pre-Release version of this extension" from the Extensions view.

image

@micahlt
Copy link

micahlt commented May 25, 2023

Thanks for the fast response! It doesn't appear to have made any difference. I switched to the prerelease version and verified that I was on 2023.5.25xx, and I changed ${branch} to ${branch_encoded}. However, nothing has changed and the opening the branch feat/calendar-embed still results in an unencoded slash.

@eamodio
Copy link
Member

eamodio commented May 25, 2023

Hrm, are you using the commands to open or copy the url?

@micahlt
Copy link

micahlt commented May 25, 2023

I'm using the command palette, so Ctrl+Shift+P and then GitLens: Open branch on remote.

@eamodio
Copy link
Member

eamodio commented May 25, 2023

Gotcha, I'll have to look into that. Do the copy commands work for you?

@micahlt
Copy link

micahlt commented May 25, 2023

That's odd, yes they do.

@micahlt
Copy link

micahlt commented May 25, 2023

It appears that only the actual Open commands are still broken.

@eamodio eamodio removed the pending-release Resolved but not yet released to the stable edition label Jun 14, 2023
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New feature or request needs-verification Request for verification
Projects
None yet
Development

No branches or pull requests

4 participants
@eamodio @amir-f @micahlt and others