frabjous / knap

Neovim plugin for creating live-updating-as-you-type previews of LaTeX, markdown, and other files in the viewer of your choice.
GNU General Public License v3.0
349 stars 10 forks source link

incompatibility on windows when lauching a viewer #27

Closed andregpss closed 4 months ago

andregpss commented 8 months ago

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.

frabjous commented 8 months ago

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.

andregpss commented 8 months ago

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

Below is the knap.lua file with the modified functionlaunch_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
frabjous commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

Good news: Knap works on Windows, including viewer autorefresh and forward jump. I tested with converting from MD to HTMLand 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?

leanhdung1994 commented 5 months ago

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 commented 4 months ago

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

andregpss commented 4 months ago

@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
frabjous commented 4 months ago

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)!