Skip to content

Conversation

@zhlinh
Copy link

@zhlinh zhlinh commented Aug 6, 2015

fix open_browser.
When shellescape(a:url) is used to substitute, it will bring a problem that the url was added with ''(single quotation marks) in https://gist.github.com/xxx, which will cause the browser use something like %27https//gist.github.com/xxx' to open a website.

@mattn
Copy link
Owner

mattn commented Aug 7, 2015

What OS do you use? For example, but not possiblely in gist.vim, query parameters with & should be escaped in arguments because it will be handled as foo & in UNIX OSs.

@zhlinh
Copy link
Author

zhlinh commented Aug 7, 2015

Sorry, Windows. And Windows can't use parameters with &. So what really make it work for me is that

if cmd =~# '^!'
     let cmd = substitute(cmd, '%URL%', '\=shellescape(a:url)', 'g')
     silent! exec cmd
elseif cmd =~# '^:[A-Z]'
     let cmd = substitute(cmd, '%URL%', '\=a:url', 'g')
     exec cmd
else
-    let cmd = substitute(cmd, '%URL%', '\=shellescape(a:url)', 'g')
+   let cmd = substitute(cmd, '%URL%', '\=a:url', 'g')
endif

@mattn
Copy link
Owner

mattn commented Aug 7, 2015

I guess it should be separated with unix part and windows part like below.

if has("win32") || has("win64")
  let cmd = substitute(cmd, '%URL%', '\=a:url', 'g')
else
  let cmd = substitute(cmd, '%URL%', '\=shellescape(a:url)', 'g')
endif

@zhlinh
Copy link
Author

zhlinh commented Aug 7, 2015

Thanks, you are right. It will be better. And I will specify the change only for Windows.

@zhlinh
Copy link
Author

zhlinh commented Aug 12, 2015

I have found a better way to solve this problem for windows.

@mattn
Copy link
Owner

mattn commented Aug 12, 2015

No, please don't change shellslash.

@zhlinh
Copy link
Author

zhlinh commented Aug 13, 2015

shellslash is recovered before this function ends. if exists('+shellslash') restricts it only works when a backslash can be used as a path.
And Windows just needs to change single quotation marks to double quotation marks.

@mattn
Copy link
Owner

mattn commented Aug 13, 2015

I don't understand why you want shellslash. what shell do you use on windows? bash on msys2?

@zhlinh
Copy link
Author

zhlinh commented Aug 13, 2015

Actually cmd.exe. Maybe just my own problem that shellslash is set. :help shellescape, and it says that:
On MS-Windows and MS-DOS, when 'shellslash' is not set, it will enclose {string} in double quotes and double all double quotes within {string}.
For other systems, it will enclose {string} in single quotes and replace all "'" with "'''".

@mattn
Copy link
Owner

mattn commented Aug 13, 2015

So, how about below?

if &shellslash
  let cmd = substitute(cmd, '%URL%', '\=shellescape(a:url)', 'g')
else
  let cmd = substitute(cmd, '%URL%', '\=a:url', 'g')
endif

@zhlinh
Copy link
Author

zhlinh commented Aug 13, 2015

If on Windows, then set shellslash, your code above may not work. Because it will enclose {string} in single quotes.
The problem is just the single or double quotes.
On Windows:
set shellslash -------------- shellescape() brings single quotes, and it doesn't work.
set noshellslash(default) -------------- shellescape() brings double quotes,and it works.

I have used other bundle vim-latex and set shellslash from its official recommended configuration. So it just could be my own problem which can be ignore. By vim default, it will work.

@mattn mattn closed this in ea7dc96 Aug 13, 2015
@zhlinh zhlinh deleted the fix_open_browser branch August 15, 2015 10:21
allex pushed a commit to allex/gist-vim that referenced this pull request May 18, 2022
allex pushed a commit to allex/gist-vim that referenced this pull request May 18, 2022
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.

2 participants