fix windows #32

README.md

@@ -52,15 +52,15 @@ Install with vim-plug:
 
 ```vim
 Plug 'kiyoon/jupynium.nvim', { 'do': 'pip3 install --user .' }
-" Plug 'kiyoon/jupynium.nvim', { 'do': '~/miniconda3/envs/jupynium/bin/pip install .' }
+" Plug 'kiyoon/jupynium.nvim', { 'do': 'conda run --no-capture-output -n jupynium pip install .' }
 Plug 'rcarriga/nvim-notify'  " optional
 ```
 
 Install with packer.nvim:
 
 ```lua
 use { "kiyoon/jupynium.nvim", run = "pip3 install --user ." }
--- use { "kiyoon/jupynium.nvim", run = "~/miniconda3/envs/jupynium/bin/pip install ." }
+-- use { "kiyoon/jupynium.nvim", run = "conda run --no-capture-output -n jupynium pip install ." }
 use { "rcarriga/nvim-notify" }  -- optional
 ```
 
@@ -70,7 +70,7 @@ Install with 💤lazy.nvim
   {
     "kiyoon/jupynium.nvim",
     build = "pip3 install --user .",
-    -- build = "~/miniconda3/envs/jupynium/bin/pip install .",
+    -- build = "conda run --no-capture-output -n jupynium pip install .",
     -- enabled = vim.fn.isdirectory(vim.fn.expand "~/miniconda3/envs/jupynium"),
   },
   "rcarriga/nvim-notify",  -- optional
@@ -85,8 +85,8 @@ Click to see the setup defaults
 
 ```lua
 require("jupynium").setup({
-  -- Conda users:
-  -- python_host = "~/miniconda3/envs/jupynium/bin/python",
+  --- For Conda environment named "jupynium",
+  -- python_host = { "conda", "run", "--no-capture-output", "-n", "jupynium", "python" },
   python_host = vim.g.python3_host_prog or "python3",
 
   default_notebook_URL = "localhost:8888",
@@ -95,7 +95,9 @@ require("jupynium").setup({
   -- When you call :JupyniumStartAndAttachToServer and no notebook is open,
   -- then Jupynium will open the server for you using this command. (only when notebook_URL is localhost)
   jupyter_command = "jupyter",
-  -- jupyter_command = "~/miniconda3/bin/jupyter",
+  --- For Conda, maybe use base environment
+  --- then you can `conda install -n base nb_conda_kernels` to switch environment in Jupyter Notebook
+  -- jupyter_command = { "conda", "run", "--no-capture-output", "-n", "base", "jupyter" },
 
   -- Used when notebook is launched by using jupyter_command.
   -- If nil or "", it will open at the git directory of the current buffer,

lua/jupynium/options.lua

@@ -2,8 +2,8 @@ local M = {}
 
 M.opts = {}
 M.default_opts = {
-  -- Conda users:
-  -- python_host = "~/miniconda3/envs/jupynium/bin/python"
+  --- For Conda environment named "jupynium",
+  -- python_host = { "conda", "run", "--no-capture-output", "-n", "jupynium", "python" },
   python_host = vim.g.python3_host_prog or "python3",
 
   default_notebook_URL = "localhost:8888",
@@ -12,7 +12,9 @@ M.default_opts = {
   -- When you call :JupyniumStartAndAttachToServer and no notebook is open,
   -- then Jupynium will open the server for you using this command. (only when notebook_URL is localhost)
   jupyter_command = "jupyter",
-  -- jupyter_command = "~/miniconda3/bin/jupyter",
+  --- For Conda, maybe use base environment
+  --- then you can `conda install -n base nb_conda_kernels` to switch environment in Jupyter Notebook
+  -- jupyter_command = { "conda", "run", "--no-capture-output", "-n", "base", "jupyter" },
 
   -- Used when notebook is launched by using jupyter_command.
   -- If nil or "", it will open at the git directory of the current buffer,

lua/jupynium/server.lua

@@ -8,52 +8,62 @@ M.server_state = {
   is_autoattached = false,
 }
 
-local function TableConcat(t1, t2)
-  for i = 1, #t2 do
-    t1[#t1 + 1] = t2[i]
-  end
-  return t1
-end
-
 local function run_process_bg(cmd, args)
   args = args or {}
   local cmd_str
   if vim.fn.has "win32" == 1 then
-    cmd_str = [[PowerShell "Start-Process -FilePath \"]]
-      .. vim.fn.expand(cmd):gsub("\\", "\\\\")
-      .. [[\" -ArgumentList \"]]
+    cmd_str = [[PowerShell "Start-Process -NoNewWindow -FilePath \"]] .. vim.fn.expand(cmd) .. [[\" -ArgumentList \"]]
 
     for _, v in ipairs(args) do
-      cmd_str = cmd_str .. [[ `\"]] .. v:gsub("\\", "\\\\") .. [[`\"]]
+      cmd_str = cmd_str .. [[ `\"]] .. v .. [[`\"]]
     end
 
     cmd_str = cmd_str .. [[\""]]
   else
-    cmd_str = [["]] .. vim.fn.expand(cmd) .. [["]]
+    cmd_str = [[']] .. vim.fn.expand(cmd) .. [[']]
 
     for _, v in ipairs(args) do
-      cmd_str = cmd_str .. [[ "]] .. v:gsub("\\", "\\\\") .. [["]]
+      -- cmd_str = cmd_str .. [[ ']] .. v:gsub("\\", "\\\\") .. [[']]
+      cmd_str = cmd_str .. [[ ']] .. v .. [[']]
     end
 
-    -- prior to nvim 0.9.0, stderr will create graphical glitch.
-    -- https://github.com/neovim/neovim/issues/21376
-    cmd_str = cmd_str .. [[ 2> /dev/null &]]
+    cmd_str = cmd_str .. [[ &]]
   end
 
-  io.popen(cmd_str)
+  vim.fn.system(cmd_str)
 end
 
 local function run_process(cmd, args)
   args = args or {}
-  local call_str = [[system('"]] .. vim.fn.expand(cmd) .. [["]]
+  local cmd_str
 
-  for _, v in ipairs(args) do
-    call_str = call_str .. [[ "]] .. v:gsub("\\", "\\\\") .. [["]]
+  if vim.fn.has "win32" == 1 then
+    if utils.string_begins_with(vim.o.shell, "powershell") then
+      cmd_str = [[& ']] .. vim.fn.expand(cmd) .. [[']]
+      for _, v in ipairs(args) do
+        cmd_str = cmd_str .. [[ ']] .. v .. [[']]
+      end
+    else
+      -- cmd.exe
+      -- Wrapping the command with double quotes means it's a file, not a command
+      -- So you need to check if you're running a command or a file.
+      cmd_str = vim.fn.expand(cmd)
+      if cmd_str:find " " ~= nil then
+        cmd_str = [["]] .. cmd_str .. [["]]
+      end
+      for _, v in ipairs(args) do
+        cmd_str = cmd_str .. [[ "]] .. v .. [["]]
+      end
+    end
+  else
+    -- linux, mac
+    cmd_str = [[']] .. vim.fn.expand(cmd) .. [[']]
+    for _, v in ipairs(args) do
+      cmd_str = cmd_str .. [[ ']] .. v .. [[']]
+    end
   end
 
-  call_str = call_str .. [[')]]
-
-  local output = vim.fn.eval(call_str)
+  local output = vim.fn.system(cmd_str)
   if output == nil then
     return ""
   else
@@ -67,14 +77,14 @@ local function call_jupynium_cli(args, bg)
     bg = true
   end
 
-  args = TableConcat({ "-m", "jupynium", "--nvim_listen_addr", vim.v.servername }, args)
+  args = utils.table_concat({ "-m", "jupynium", "--nvim_listen_addr", vim.v.servername }, args)
 
   local cmd
   if type(options.opts.python_host) == "string" then
     cmd = options.opts.python_host
   elseif type(options.opts.python_host) == "table" then
     cmd = options.opts.python_host[1]
-    args = TableConcat({ unpack(options.opts.python_host, 2) }, args)
+    args = utils.table_concat({ unpack(options.opts.python_host, 2) }, args)
   else
     error "Invalid python_host type."
   end

lua/jupynium/utils.lua

@@ -44,4 +44,11 @@ function M.remove_duplicates(list)
   return res
 end
 
+function M.table_concat(t1, t2)
+  for i = 1, #t2 do
+    t1[#t1 + 1] = t2[i]
+  end
+  return t1
+end
+
 return M

src/jupynium/cmds/jupynium.py

@@ -6,6 +6,7 @@
 import logging
 import os
 import secrets
+import signal
 import subprocess
 import sys
 import tempfile
@@ -17,6 +18,7 @@
 import coloredlogs
 import git
 import persistqueue
+import psutil
 import verboselogs
 from git.exc import InvalidGitRepositoryError
 from persistqueue.exceptions import Empty
@@ -164,8 +166,7 @@ def get_parser():
         nargs="+",
         default=["jupyter"],
         help="Command to start Jupyter Notebook (but without notebook).\n"
-        "To use conda env, use `--jupyter_command ~/miniconda3/envs/env_name/bin/jupyter`.\n"
-        "Don't use `conda run ..` as it won't be killed afterwards (it opens another process with different pid so it's hard to keep track of it.)\n"
+        "To use conda env, use `--jupyter_command conda run ' --no-capture-output' ' -n' base jupyter`. Notice the space before the dash.\n"
         "It is used only when the --notebook_URL is localhost, and is not running.",
     )
     parser.add_argument(
@@ -281,16 +282,39 @@ def exception_no_notebook(notebook_URL, nvim):
     sys.exit(1)
 
 
+def kill_child_processes(parent_pid, sig=signal.SIGTERM):
+    try:
+        parent = psutil.Process(parent_pid)
+    except psutil.NoSuchProcess:
+        return
+    children = parent.children(recursive=True)
+    for process in children:
+        process.send_signal(sig)
+    psutil.wait_procs(children, timeout=3)
+
+
 def kill_notebook_proc(notebook_proc):
     """
     Kill the notebook process.
     Used if we opened a Jupyter Notebook server using the --jupyter_command and when no server is running.
     """
     if notebook_proc is not None:
-        notebook_proc.terminate()
-        # notebook_proc.kill()
-        notebook_proc.wait()
-        logger.info("Jupyter Notebook server has been killed.")
+        if os.name == "nt":
+            # Windows
+            os.kill(notebook_proc.pid, signal.CTRL_C_EVENT)
+        else:
+            # Twice to properly close
+            kill_child_processes(notebook_proc.pid, signal.SIGINT)
+            kill_child_processes(notebook_proc.pid, signal.SIGINT)
+
+            ## Below doesn't work when the notebook is started like
+            ## conda run --no-capture-output -n base jupyter notebook
+            # notebook_proc.terminate()
+            # # notebook_proc.kill()
+            # notebook_proc.wait()
+        logger.info(
+            f"Jupyter Notebook server (pid={notebook_proc.pid}) has been killed."
+        )
 
 
 def fallback_open_notebook_server(
@@ -330,7 +354,6 @@ def fallback_open_notebook_server(
     try:
         # strip commands because we need to escape args with dashes.
         # e.g. --jupyter_command conda run ' --no-capture-output' ' -n' env_name jupyter
-        # However, conda run will run process with another pid so it won't work well here. Don't use it.
 
         jupyter_command = [command.strip() for command in jupyter_command]
         jupyter_command[0] = os.path.expanduser(jupyter_command[0])

src/jupynium/events_control.py

@@ -165,7 +165,11 @@ def process_events(nvim_info: NvimInfo, driver):
                 return False, request_event
         else:
             # process and update prev_lazy_args
-            process_notification_event(nvim_info, driver, event, prev_lazy_args_per_buf)
+            status = process_notification_event(
+                nvim_info, driver, event, prev_lazy_args_per_buf
+            )
+            if not status:
+                return False, None
 
     # After the loop (here) you need to process the last on_lines event.
     prev_lazy_args_per_buf.process_all(nvim_info, driver)
@@ -327,6 +331,7 @@ def process_request_event(nvim_info: NvimInfo, driver, event):
         logger.info(f"Loaded ipynb to the nvim buffer.")
 
     elif event[1] == "VimLeavePre":
+        # For non-Windows, use rpcrequest
         logger.info("Nvim closed. Clearing nvim")
         return False, event[3]
 
@@ -412,7 +417,7 @@ def process_notification_event(
     assert event[0] == "notification"
 
     if skip_bloated(nvim_info):
-        return
+        return True
 
     bufnr = event[2][0]
     event_args = event[2][1:]
@@ -540,11 +545,17 @@ def process_notification_event(
             logger.info(f"Received stop_sync request: bufnr = {bufnr}")
             nvim_info.detach_buffer(bufnr, driver)
 
+        elif event[1] == "VimLeavePre":
+            # Only for Windows, use rpcnotify
+            logger.info("Nvim closed. Clearing nvim")
+            return False
+
+    return True
+
 
 def update_cell_selection(
     nvim_info: NvimInfo, driver, bufnr, update_selection_args: UpdateSelectionArgs
 ):
-
     cursor_pos_row, visual_start_row = dataclasses.astuple(update_selection_args)
 
     if nvim_info.jupbufs[bufnr].num_cells == 1:

src/jupynium/lua/autocmd_vimleave.lua

@@ -2,7 +2,12 @@ local augroup = vim.api.nvim_create_augroup("jupynium_global", { clear = true })
 vim.api.nvim_create_autocmd({ "VimLeavePre" }, {
   -- Don't set the buffer. You can leave from another file.
   callback = function()
-    Jupynium_rpcrequest("VimLeavePre", 0)
+    if vim.fn.has "win32" == 1 then
+      -- On Windows, when the VimLeavePre event is triggered, the rpc is not able to respond to the request.
+      Jupynium_rpcnotify("VimLeavePre", 0)
+    else
+      Jupynium_rpcrequest("VimLeavePre", 0)
+    end
   end,
   group = augroup,
 })