fix: defer closing guest to make rpcrequest work #102

lostl1ght · 2024-07-20T01:41:57Z

Closes #101

So, I've solved it. The guest was quitting too fast thus rendering the channel invalid. Deferring the quit command allowed the vim.rpcrequest function to be used.

I've also increased sleep time to 10s as I proposed in #101 but I can remove this commit if you don't like it.

lostl1ght · 2024-12-23T15:36:33Z

Hi, @willothy! It would be amazing if you could take a look at the PR. It's been almost half a year with no feedback.

willothy · 2024-12-23T18:29:47Z

Hi, @willothy! It would be amazing if you could take a look at the PR. It's been almost half a year with no feedback.

I'm sorry! I've been quite busy this year and haven't been able to prioritize plugin development. I'll try this out right now :)

willothy · 2024-12-23T18:31:56Z

lua/flatten/core.lua

    response_sock,
    "nvim_exec_lua",
-    ---@diagnostic disable-next-line: param-type-mismatch
-    "vim.cmd.qa({ bang = true })",
+    "vim.defer_fn(function() vim.cmd.qa({ bang = true }) end, 25)",


Have you tried this with just vim.schedule? I'd like to avoid timers here if possible.

I don't remember why I've used defer_fn instead of schedule but schedule seems to work as well.

willothy · 2024-12-23T18:32:50Z

lua/flatten/guest.lua

@@ -22,7 +22,7 @@ function M.maybe_block(block)
  end
  vim.fn.chanclose(host)
  while true do
-    vim.cmd.sleep(1)
+    vim.cmd.sleep(10)


I think maybe this should just be changed to vim.wait - I will look into this now. I wrote the original code for this quite a while ago when I was a lot less familiar with Neovim's internals and APIs.

There's a big problem with vim.wait - it is async, meaning guest stays fully responsive which is what we do not want because a user can accidentally mess up guest. sleep on the other hand fully blocks guest neovim instance.

Hmm I'm not sure this is the case - vim.wait is blocking (for Lua), it just may not fully block the event loop - I don't think the guest is responsive while blocked. It definitely isn't for me, main is using vim.wait now and it works for me :)

It is blocking the function where it is called but it is not blocking the UI.

Try this:

Open terminal: :term

Add something to index

git commit

Return to the terminal buffer in host

i once, you will enter terminal mode in host <- this is expected

i second time, you will enter insert mode in guest <- this should not happen but it does because vim.wait only blocks maybe_block

That's why I believe it is better to use sleep.

willothy · 2024-12-23T18:48:09Z

Also, have you tried the guest-data branch from #86? That's what I'm using, I just haven't found the time to finish it up and merge tbh

willothy · 2024-12-23T20:05:14Z

I've just merged that PR - mind checking if the issue is still present in main?

lostl1ght · 2024-12-24T01:58:22Z

I'm a bit confused. Guest is not getting blocked.

For this test I'm using the default config:

require('flatten').setup({})

Lazygit config:

os:
  editPreset: nvim

Open a terminal with :terminal
Open lazygit in the terminal
Add something into index
Try committing with editor: C
lazygit immediately exits with Aborting commit due to empty commit message. error.

COMMIT_EDITMSG file is getting opened in host.

Am I missing something?

willothy · 2024-12-24T05:00:18Z

Am I missing something?

Nope, I accidentally introduced a bug earlier today. Should be fixed now! I didn't catch it because it was only breaking with open = 'current'.

lostl1ght · 2024-12-24T15:12:22Z

I don't think we need unblock in guest as it introduces a delay (up to 1s in this case) before closing guest which is not fun

Based on this comment and the comments before above I've adjusted unblock_guest and maybe_block as below:

Diff

diff --git a/lua/flatten/core.lua b/lua/flatten/core.lua
index 151df18..e03f3a7 100644
--- a/lua/flatten/core.lua
+++ b/lua/flatten/core.lua
@@ -9,10 +9,10 @@ function M.unblock_guest(guest_pipe)
     })
     return
   end
-  vim.rpcnotify(
+  vim.rpcrequest(
     response_sock,
     "nvim_exec_lua",
-    "require('flatten.guest').unblock()",
+    "vim.schedule(function() vim.cmd.qa({ bang = true }) end)",
     {}
   )
   if vim.api.nvim_get_chan_info(response_sock).id ~= nil then
diff --git a/lua/flatten/guest.lua b/lua/flatten/guest.lua
index 9281a76..536f29a 100644
--- a/lua/flatten/guest.lua
+++ b/lua/flatten/guest.lua
@@ -2,7 +2,6 @@ local M = {}

 local config = require("flatten").config

-local waiting = false
 local host

 local function is_windows()
@@ -17,20 +16,13 @@ function M.exec_on_host(call, opts)
   return vim.rpcrequest(host, "nvim_exec_lua", call, opts or {})
 end

-function M.unblock()
-  waiting = false
-end
-
 function M.maybe_block(block)
   if not block then
     vim.cmd.qa({ bang = true })
   end
-  waiting = true
-  vim.wait(math.huge, function()
-    return waiting == false
-      or vim.api.nvim_get_chan_info(host) == vim.empty_dict()
-  end, 1000)
-  vim.cmd.qa({ bang = true })
+  while true do
+    vim.cmd("sleep 10")
+  end
 end

 function M.send_files(files, stdin)

But sometimes terminal colors get messed up and I get flashbanged by a light color:

Bright image

I can't figure out yet why that happens.

willothy · 2024-12-24T15:47:18Z

Hmm, I'm not experiencing the delay or the flashbang. What terminal emulator are you using?

Edit: Try 4258a0a, maybe that change will fix the delay :)

lostl1ght · 2024-12-24T16:28:14Z

There's something wrong with using math.huge in vim.wait. Guest produces this error:

Changing timeout to an actual integer fixes the error.

Probably because vim.wait is C API and C does not have a representation for infinity.

lostl1ght · 2024-12-24T16:35:33Z

Edit: Try 4258a0a, maybe that change will fix the delay :)

That only decreases the delay to 200ms, which is better but not instant. And it puts pressure on the guest to process the function call every 200ms. This call is not a particularly expensive operation but it still costs some CPU time. Where sleep + sending :qa! does not cost anything more and unblocking is instant and guest is fully blocked.

lostl1ght · 2024-12-24T16:42:54Z

Hmm, I'm not experiencing the delay or the flashbang. What terminal emulator are you using?

It's alacritty 0.13 + tmux 3.5a. I'll try to figure out what is causing it and report later.

willothy · 2024-12-24T16:49:59Z

Edit: Try 4258a0a, maybe that change will fix the delay :)

That only decreases the delay to 200ms, which is better but not instant. And it puts pressure on the guest to process the function call every 200ms. This call is not a particularly expensive operation but it still costs some CPU time. Where sleep + sending :qa! does not cost anything more and unblocking is instant and guest is fully blocked.

Is the 200ms noticeable? If it is we could just decrease it to like 100ms or 50ms. I think the cost of running that function every 100ms even would be negligible. vim.wait is blocking as well but allows more control over interrupting the wait, so I'd like to keep it here - it also works as expected for me, at least.

willothy · 2024-12-24T17:17:25Z

Probably because vim.wait is C API and C does not have a representation for infinity.

Good point, this one definitely will have weird behavior depending on the platform - changed it to just be 0xffffffffffffffff

lostl1ght force-pushed the fix-freezing-2 branch from 6e3a099 to a1cfd1b Compare July 20, 2024 01:46

lostl1ght added 2 commits July 20, 2024 04:56

fix: defer closing guest to make rpcrequest work

a73b7a7

perf: increase sleep time to 10s

62a1976

lostl1ght force-pushed the fix-freezing-2 branch from a1cfd1b to 62a1976 Compare July 20, 2024 01:57

willothy reviewed Dec 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: defer closing guest to make rpcrequest work #102

fix: defer closing guest to make rpcrequest work #102

lostl1ght commented Jul 20, 2024 •

edited

Loading

lostl1ght commented Dec 23, 2024

willothy commented Dec 23, 2024

willothy Dec 23, 2024

lostl1ght Dec 24, 2024

willothy Dec 23, 2024

lostl1ght Dec 24, 2024

willothy Dec 24, 2024

lostl1ght Dec 24, 2024

willothy commented Dec 23, 2024

willothy commented Dec 23, 2024

lostl1ght commented Dec 24, 2024

willothy commented Dec 24, 2024

lostl1ght commented Dec 24, 2024

willothy commented Dec 24, 2024 •

edited

Loading

lostl1ght commented Dec 24, 2024 •

edited

Loading

lostl1ght commented Dec 24, 2024 •

edited

Loading

lostl1ght commented Dec 24, 2024

willothy commented Dec 24, 2024

willothy commented Dec 24, 2024

fix: defer closing guest to make rpcrequest work #102

Are you sure you want to change the base?

fix: defer closing guest to make rpcrequest work #102

Conversation

lostl1ght commented Jul 20, 2024 • edited Loading

lostl1ght commented Dec 23, 2024

willothy commented Dec 23, 2024

willothy Dec 23, 2024

Choose a reason for hiding this comment

lostl1ght Dec 24, 2024

Choose a reason for hiding this comment

willothy Dec 23, 2024

Choose a reason for hiding this comment

lostl1ght Dec 24, 2024

Choose a reason for hiding this comment

willothy Dec 24, 2024

Choose a reason for hiding this comment

lostl1ght Dec 24, 2024

Choose a reason for hiding this comment

willothy commented Dec 23, 2024

willothy commented Dec 23, 2024

lostl1ght commented Dec 24, 2024

willothy commented Dec 24, 2024

lostl1ght commented Dec 24, 2024

willothy commented Dec 24, 2024 • edited Loading

lostl1ght commented Dec 24, 2024 • edited Loading

lostl1ght commented Dec 24, 2024 • edited Loading

lostl1ght commented Dec 24, 2024

willothy commented Dec 24, 2024

willothy commented Dec 24, 2024

lostl1ght commented Jul 20, 2024 •

edited

Loading

willothy commented Dec 24, 2024 •

edited

Loading

lostl1ght commented Dec 24, 2024 •

edited

Loading

lostl1ght commented Dec 24, 2024 •

edited

Loading