search

ruanyl / vim-gh-line

vim plugin that open the link of current line on github
MIT License
408 stars 37 forks source link

Make sure colon in the https:// scheme is not replaced #17

Closed ahakanbaba closed 5 years ago

ahakanbaba commented 5 years ago

fixes #16

KostyaEsmukov commented 5 years ago

Unfortunately your solution doesn't seem to work on my system with neither GNU nor BSD sed (I expect that the : should be turned to /):

bash-3.2$ echo git@github.com:ruanyl/vim-gh-line.git | gsed 's/(https)\@<!:/\//'
git@github.com:ruanyl/vim-gh-line.git
bash-3.2$ echo git@github.com:ruanyl/vim-gh-line.git | sed 's/(https)\@<!:/\//'
git@github.com:ruanyl/vim-gh-line.git

bash-3.2$ echo git@github.com:ruanyl/vim-gh-line.git | gsed 's/\(https\)\@<!:/\//'
git@github.com:ruanyl/vim-gh-line.git
bash-3.2$ echo git@github.com:ruanyl/vim-gh-line.git | sed 's/\(https\)\@<!:/\//'
git@github.com:ruanyl/vim-gh-line.git

Maybe I'm doing the shell/sed escapes wrong? Could you give any reference to the syntax of the negative lookbehind expression for sed that you're using? I couldn't find any.

However, it's possible to avoid lookbehinds completely for this problem by combining 1st and 2nd substitutions:

bash-3.2$ echo git@github.com:ruanyl/vim-gh-line.git | gsed 's/^[^@:]*@\([^:]*\):/https:\/\/\1\//'
https://github.com/ruanyl/vim-gh-line.git
bash-3.2$ echo git@github.com:ruanyl/vim-gh-line.git | sed 's/^[^@:]*@\([^:]*\):/https:\/\/\1\//'
https://github.com/ruanyl/vim-gh-line.git

bash-3.2$ echo https://github.com/ruanyl/vim-gh-line.git | sed 's/^[^@:]*@\([^:]*\):/https:\/\/\1\//'
https://github.com/ruanyl/vim-gh-line.git
bash-3.2$ echo https://github.com/ruanyl/vim-gh-line.git | gsed 's/^[^@:]*@\([^:]*\):/https:\/\/\1\//'
https://github.com/ruanyl/vim-gh-line.git

PS: It would be nice to set up regression tests and CI for this project (obviously not in this PR, but in general).

ahakanbaba commented 5 years ago

Thank you @KostyaEsmukov for your input. I have used the regular expressions you suggested. The need for forward-backward slash escaping is rather unfortunate but I think this should work. It did in my setup.

KostyaEsmukov commented 5 years ago

@ahakanbaba Thanks! I think that will do.

ahakanbaba commented 5 years ago

@ruanyl could you please review this PR ? It is a bugfix and it would be good to merge it sooner than later.

ruanyl commented 5 years ago

thank you very much for the pr, works good with Github/Bitbucket https and ssh remote url.