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

fix the problem that open viewer in windows/neovim will show a cmd pr… #2627

Closed
wants to merge 1 commit into from

Conversation

archerc
Copy link

@archerc archerc commented Feb 5, 2023

In the case that runing on a Windows / neovim installation, the vimtex general viewer will open the viewer with a command prompt shown instead of open it silently. I found that this is because the job run a command with a prefix "cmd /s /c " or in a list with head ["sh", "-c" ]. This may be fine on other platforms but not in my case. I want to prefix the command with "start " so I need a place to customize it as in this pull request.

@lervag
Copy link
Owner

lervag commented Feb 5, 2023

In the case that runing on a Windows / neovim installation, the vimtex general viewer will open the viewer with a command prompt shown instead of open it silently. I found that this is because the job run a command with a prefix "cmd /s /c " or in a list with head ["sh", "-c" ]. This may be fine on other platforms but not in my case. I want to prefix the command with "start " so I need a place to customize it as in this pull request.

Thanks, I appreciate the effort of looking into this in depth and suggesting an update!

You propose this (I've made a few minor tweaks for readability):

function! vimtex#jobs#neovim#new(cmd) abort " {{{1
  let l:job = deepcopy(s:job)

  let l:modifier = get(g:, 'vimtex_job_cmd_modifier',
        \ has('win32') ? 'start ' : '')
  if empty(l:modifier)
    let l:job.cmd = has('win32')
          \ ? 'cmd /s /c "' . a:cmd . '"'
          \ : ['sh', '-c', a:cmd]
  elseif type(l:modifier) == v:t_func
        \ || (type(l:modifier) == v:t_string && exists('*' . l:modifier))
    let l:job.cmd = call(l:modifier, a:cmd)
  else
    let l:job.cmd = l:modifier . a:cmd
  endif

  return l:job
endfunction

So:

  • You are changing the current default behaviour on Windows. I don't actually mind that, but it does need some thorough testing! I don't use Windows, so it means you (and possibly others) need to do this testing. E.g., does this break any current behaviour that uses the jobs api?
  • You are making this change only for neovim, but we should also consider the Vim version: https://github.com/lervag/vimtex/blob/master/autoload/vimtex/jobs/vim.vim#L7-L13.
  • Do we really need the new option? I would be much more happy if we can instead find something that "just works"™ for everyone. If start is a better way to run system commands on Windows than cmd /s /c, then I won't mind just changing this at the core without any options.
  • If we want the option: It needs to be properly documented.

VimTeX uses vimtex#jobs#neovim#new through vimtex#jobs#start, which us currently used for starting the generic viewer, Sioyek (viewer), and for opening a browser or similar for handling viewing documentation. There are some other system call api functions that works slightly differently, but that also uses cmd /s /c on Windows. Does start work well as a substitute for cmd /s /c for every type of system call?

@lervag
Copy link
Owner

lervag commented May 24, 2023

I'm curious if #2717 resolves this issue?

lervag added a commit that referenced this pull request Jun 8, 2023
This is because, for some reason, detaching leads to a popup command
window that is annoying to users.

refer: #2401, #2627, #2706, #2717
@lervag lervag closed this Jun 8, 2023
@lervag
Copy link
Owner

lervag commented Jun 8, 2023

I believe this is resolved by #2717 and I'm closing this due to otherwise inactivity.

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