mattn / vim-gist

Vim plugin for Gist
http://www.vim.org/scripts/script.php?script_id=2423
1.69k stars 131 forks source link

fix open_browser #196

Closed zhlinh closed 9 years ago

zhlinh commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

zhlinh commented 9 years ago

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

mattn commented 9 years ago

No, please don't change shellslash.

zhlinh commented 9 years ago

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 commented 9 years ago

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

zhlinh commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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.