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

incompatibility on windows when lauching a viewer #27

Closed
andregpss opened this issue Feb 21, 2024 · 9 comments
Closed

incompatibility on windows when lauching a viewer #27

andregpss opened this issue Feb 21, 2024 · 9 comments

Comments

@andregpss
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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

@frabjous
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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?

@leanhdung1994
Copy link

leanhdung1994 commented May 26, 2024

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
Copy link

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

@andregpss
Copy link
Contributor Author

@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
Copy link
Owner

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

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

No branches or pull requests

3 participants