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
328 stars 9 forks source link

Launching Preview on OSX Sonoma #33

Open Crenshinibon opened 4 weeks ago

Crenshinibon commented 4 weeks ago

With NeoVim (v0.10.0), Lazy and the latest knap version I get the following error when trying to lunch the preview:

E5108: Error executing lua: /Users/porsched/.local/share/nvim/lazy/knap/lua/knap.lua:134: attempt to call upvalue 'err_msg' (a nil value)
stack traceback:
        /Users/porsched/.local/share/nvim/lazy/knap/lua/knap.lua:134: in function 'get_os'
        /Users/porsched/.local/share/nvim/lazy/knap/lua/knap.lua:137: in main chunk
        [C]: in function 'require'
        /Users/porsched/.config/nvim/init.lua:157: in function </Users/porsched/.config/nvim/init.lua:156>

It seems as if this function is the problem:

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
        err_msg("Unknown operating system")
    end
end

When I change it like this:

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 "linux"
        --err_msg("Unknown operating system")
    end
end

I don't get the error and the preview is opening up as expected.

andregpss commented 4 weeks ago

I found that when using macOS, vim.loop.os_uname().sysname returns Darwin value. Can you confirm? So, the following code must work in your machine:

function get_os()

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

    if os_current == "Linux" or os_current == "Darwin" then
        return "linux"
    elseif isWindows ~= nil then
        return "windows"
    else
        err_msg("Unknown operating system")
    end
end
Crenshinibon commented 4 weeks ago

Yes, this does work.

Crenshinibon commented 4 weeks ago

I'm not sure about the context of this function. But from the distance it's main purpose is to decide if it's Windows and then do something special.

Maybe the following should be enough, then? What else is there other then "linux" (including MacOS) and "windows" where anyone would run neovim on.

function get_os()

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

    if isWindows ~= nil then
        return "windows"
    end
    return "linux" 

end
frabjous commented 4 weeks ago

This code was introduced in a PR by a Windows user, and yes, at the moment only Windows is treated differently from Linux. I believe it would make more sense for the get_os function to return "macos" for "Darwin", and then where os_cur is used elsewhere in the code either make it disjunctive (os_cur == "windows" or os_cur == "macos") if macos and linux can be treated the same, or else handle macos differently if that's better.

Since I have no easy access to mac or windows machines, it would be best if someone who did made the changes and tested the code.

Crenshinibon commented 4 weeks ago

So actually it's not the question of the current os (when windows is the exception here and the rest behaves the same), but rather if it's windows or not. So maybe the os_cur thing should be is_windows ... just an idea. I have easy access to osx and linux but not so easy to windows (only on my sons gaming machine) ... but if I find the time, I might look into it.

andregpss commented 4 weeks ago

I also do not have access to macOS. I prefer the @frabjous idea of identifying the OS system in use. It can be useful in the future.