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

Support git+ssh:// URL #50

Merged
merged 2 commits into from
Jul 25, 2024
Merged

Support git+ssh:// URL #50

merged 2 commits into from
Jul 25, 2024

Conversation

a-m-s
Copy link

@a-m-s a-m-s commented Jul 25, 2024

For reasons I don't fully understand, my company uses remote URLs like git+ssh://user@gitlab.server.com/repo which Git and vim-fugitive are perfectly happy with, but this plugin does not recognise.

All is well with this patch.

@shumphrey
Copy link
Owner

Could you add a test case in the test/ directory?
I think that git+ssh url is one form of an ssh url, I do not believe it means you have to use that form in your config.
Also, I think this form should already work based on these test cases

Execute (gitlab#fugitive#homepage_for_remote - ssh://git@ url):
let expected = 'https://my.gitlab.com/shumphrey/fugitive-gitlab.vim'
let url = gitlab#fugitive#homepage_for_remote('ssh://git@my.gitlab.com/shumphrey/fugitive-gitlab.vim.git')
AssertEqual url, expected

@shumphrey
Copy link
Owner

shumphrey commented Jul 25, 2024

Ah no I see those cases are different.
Could you add a test case for it and if that passes I think it looks good
test should live in test/homepage_for_remote.vader

@a-m-s
Copy link
Author

a-m-s commented Jul 25, 2024

It definitely doesn't work or I wouldn't have found myself here. ;-)

I've added some testcases, but I couldn't test them locally without a bunch of set-up work. Hopefully they're good.

@shumphrey shumphrey merged commit 838d3a1 into shumphrey:master Jul 25, 2024
2 checks passed
@shumphrey
Copy link
Owner

Merged, thanks for the patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants