knap icon indicating copy to clipboard operation
knap copied to clipboard

incompatibility on windows when lauching a viewer

Open andregpss opened this issue 1 year ago • 5 comments

I found Knap very interesting, so I installed it on Windows, even though I knew it was incompatible. I verified that the processing is done successfully using pandoc. However, the problem is launching the viewer. At this point, knap shows the error Coud not launch viewer.

Analyzing Knap's launch_viewer() function, I believe that what makes it incompatible on Windows is adding the command > /dev/null 2>&1 & echo $! to the lcmd variable. The equivalent on Windows is >NUL 2>&1 & echo $!.

Can you change this, detecting the operating system in use to run the correct command? I'm looking forward to testing it.

andregpss avatar Feb 21 '24 15:02 andregpss

Someone else would have to do that. I don't have a Windows machine to test on, and have no desire to obtain one. If you're right, it doesn't sound particularly hard. However, I'm not going to put out code that hasn't been tested.

There's other code I can't imagine would be compatible, such as in the is_running() function.

frabjous avatar Feb 21 '24 16:02 frabjous

I modified the code, and it worked! The launcher now runs on Windows.

Below is the knap.lua file with the modified function launch_viewer and with get_os() function. I haven't yet tested whether the other knap functions are working or the is_running() function you mentioned.

function get_os()
  local os_current = vim.loop.os_uname().sysname
  local isWindows,j = string.find(os_current,"Windows")

  if os_current == "Linux" then
    return "linux"
  elseif isWindows ~= nil then
    return "windows"
  else
    return "unknown"
  end
end
-- run the specified command to open the viewing application
function launch_viewer()
    local os = get_os()
    local lcmd = ""
    if os == "linux" then
        -- launch viewer in background and echo pid
        lcmd = vim.b.knap_viewer_launch_cmd .. ' > /dev/null 2>&1 & echo $!'
    elseif os == "windows" then
        lcmd = vim.b.knap_viewer_launch_cmd .. ' > NUL 2>&1 & echo $!'
    else
        err_msg("Unknown operating system")
    end 
    if (vim.b.knap_docroot) then
        lcmd = 'cd "' .. dirname(vim.b.knap_docroot) .. '" && ' .. lcmd
    end
    local lproc = io.popen(lcmd)
    -- try to read pid
    local vpid = lproc:read()
    lproc:close()
    -- if couldn't read pid then it was a failure
    if not (vpid) or (vpid == '') then
        err_msg("Could not launch viewer.")
        mark_viewer_closed()
        return
    end
    -- set variables for viewer
    vim.b.knap_viewerpid = tonumber(vpid)
    vim.b.knap_viewer_launched = 1
    -- set viewer refresh command
    local vwrrefcmd = vim.b.knap_settings[vim.b.knap_routine ..
        'viewerrefresh'] or 'none';
    vim.b.knap_viewer_refresh_cmd = fill_in_cmd(vwrrefcmd)
end

andregpss avatar Feb 21 '24 18:02 andregpss

Did you want to do a pull request?

You can probably simplify the code just by running the os check once at load time, and have it set a global variable either to 'NIL' or '/dev/null' and just have the individual commands stick one or another where it needs to go, since that looks like the only difference rather than doing a if then every time.

frabjous avatar Feb 21 '24 21:02 frabjous

I noticed that the viewer does not run in background. NVim remains frozen until the viewer is closed. It seems that io.popen frozes. The lcdm variable has the value: cd "." && falkon "C:\hls\README.html" > NUL 2>&1 & echo $! When I run that command in normal Neovim mode, falkon runs in the background, as expected.

andregpss avatar Feb 22 '24 03:02 andregpss

Good news: Knap works on Windows, including viewer autorefresh and forward jump. I tested with converting from MD to HTML and PDF, and from TEX to PDF. The HTML viewer is Falkon, and the PDF viewer is Sioyek. When converting TEX to PDF, I did not test the use of Rubber. For Knap to work, I mainly had to change the creation of the viewer process and obtaining its PID. The only feature that doesn't work is the inverse search in Sioyek. I tested the Knap standard value for textopdfviewerlaunch variable, but there was an error where KnapHelper was called. Can you please show an example of how to call this module? Regarding code changes, can I make a pull request?

andregpss avatar Feb 28 '24 02:02 andregpss

Did you want to do a pull request?

You can probably simplify the code just by running the os check once at load time, and have it set a global variable either to 'NIL' or '/dev/null' and just have the individual commands stick one or another where it needs to go, since that looks like the only difference rather than doing a if then every time.

@andregpss please make a pull request as suggested by @frabjous. This is hugely beneficial toward a Windows version.

leanhdung1994 avatar May 26 '24 04:05 leanhdung1994

@andregpss Thank you so much for making the pull request. Could you please write an instruction on how to setup knap on Windows?

leanhdung1994 avatar May 28 '24 06:05 leanhdung1994

@andregpss Thank you so much for making the pull request. Could you please write an instruction on how to setup knap on Windows?

There's no additional setup on Windows besides the one described on Knap README. However, some of the software listed have no Windows version.

The Knap setting below is the only one I have used:

local gknapsettings = {
    texoutputext = "pdf",
    textopdf = "pdflatex -interaction=batchmode -halt-on-error -synctex=1 %docroot%",
    textopdfviewerrefresh = "none",
    textopdfviewerlaunch = "sioyek --inverse-search \"nvim --headless -es\"  --new-window %outputfile%",
    textopdfviewerrefresh = "none",
     textopdfforwardjump = "sioyek --inverse-search \"nvim --headless -es\" --reuse-window --forward-search-file %srcfile% --forward-search-line %line% %outputfile%",   
    textopdfshorterror = "A=%outputfile% ; LOGFILE=\"${A%.pdf}.log\"" }

    vim.g.knap_settings = gknapsettings

andregpss avatar May 28 '24 14:05 andregpss

I merged your PR, though there were some things I needed to fix, as you had 'start ' at the beginning of the launch command, which only works on Windows as far as I know, and you also lost the ' & echo $!' at the end of the launch command, which meant that it launched in the foreground and locked up neovim.

I'm just assuming it behaves properly on Windows now, as I cannot test that. It is surprising to me that you don't need a '&' at the end of the launch command.

Incidentally, I don't even use neovim anymore, and therefore not this plugin either. It would be great if someone else was interested in taking over (you can change the name)!

frabjous avatar May 28 '24 15:05 frabjous