Skip to content

Conversation

wroyca
Copy link
Contributor

@wroyca wroyca commented Sep 7, 2024

Use explicit targeting to sync the terminal background.

Nvim changes standard streams to non-blocking mode, so we should use io.stdout:write to ensure that writes are handled properly (that is, directly addressing the term's standard output).

This should address the last remaining bits from #1111:

That's both good (it fixes something) and confusing (it introduces very similar issue for me) news. I'll just focus on the first interpretation :)

Let me know how it goes :)

@echasnovski
Copy link
Member

Thanks for the PR!

Nvim changes standard streams to non-blocking mode, so we should use io.stdout:write to ensure that writes are handled properly (that is, directly addressing the term's standard output).

Could you, please, share a link to where this information coming from?

This should address the last remaining bits from #1111:

That's both good (it fixes something) and confusing (it introduces very similar issue for me) news. I'll just focus on the first interpretation :)

Let me know how it goes :)

Did you manage to reproduce similar issue as I described there? And this change should fix it, right?

@echasnovski
Copy link
Member

echasnovski commented Sep 7, 2024

I've just tried reproducing "<C-z> -> fg -> `<C-z> -> ... -> <C-z> (background is the same as in Neovim)" with and without the change in the PR. Both times I was able to reproduce. So I don't think this fixes the issue.

@wroyca wroyca closed this Sep 7, 2024
@wroyca
Copy link
Contributor Author

wroyca commented Sep 8, 2024

Thanks for the PR!

Nvim changes standard streams to non-blocking mode, so we should use io.stdout:write to ensure that writes are handled properly (that is, directly addressing the term's standard output).

Could you, please, share a link to where this information coming from?

This should address the last remaining bits from #1111:

That's both good (it fixes something) and confusing (it introduces very similar issue for me) news. I'll just focus on the first interpretation :)

Let me know how it goes :)

Did you manage to reproduce similar issue as I described there? And this change should fix it, right?

@echasnovski Sorry, didn't see that comment yesterday - oops.

Could you, please, share a link to where this information coming from?

This issue stems from a problem I encountered with build2, which only occurred when Neovim was started in the background. The build2 team investigated and discovered that non-blocking streams were the cause. For background, see build2/build2#417

Did you manage to reproduce similar issue as I described there? And this change should fix it, right?

No — I’m unable to reproduce the sync issue at all, so it’s surprising that you can reproduce it on your machine. Why is that? What’s the difference? Who knows, haha.

That said, there is a subtle difference between io.write and io.stdout:write when output redirection is involved. io.write writes to the current default output stream, which can be redirected. If the default output stream changes, io.write will respect that redirection. io.stdout:write, on the other hand, always writes to stdout, regardless of any redirection. It specifically targets the stdout stream, which always goes to the terminal (or the original standard output).

I think this change is worthwhile if only for that, even though I had hoped it would fix the sync issue on your side, but alas. Let me know if you’d like to reopen this, in any case. Reopening for now to address new points, but feel free to close it again. :)

@wroyca
Copy link
Contributor Author

wroyca commented Sep 8, 2024

Adding to this, here’s an example to illustrate the difference:

local file = io.open("output.txt", "w")
io.output(file)
file:close()

In this case, file:close will cause mini.misc (io.write) to error out with:

Error executing lua callback: .../wroy/.local/share/nvim/lazy/mini.nvim/lua/mini/misc.lua:329: standard file is closed  

But io.stdout:write will still work as expected, since it directly writes to stdout, "bypassing" the redirection and file closure.

@wroyca wroyca reopened this Sep 8, 2024
@echasnovski
Copy link
Member

Adding to this, here’s an example to illustrate the difference:

local file = io.open("output.txt", "w")
io.output(file)
file:close()

In this case, file:close will cause mini.misc (io.write) to error out with:

Error executing lua callback: .../wroy/.local/share/nvim/lazy/mini.nvim/lua/mini/misc.lua:329: standard file is closed  

But io.stdout:write will still work as expected, since it directly writes to stdout, "bypassing" the redirection and file closure.

Oh, my... You should have started with that. I can reproduce. Will schedule to finish and merge this PR tomorrow.

@echasnovski echasnovski changed the base branch from main to backlog September 9, 2024 09:11
@echasnovski echasnovski merged commit 2187376 into nvim-mini:backlog Sep 9, 2024
10 of 20 checks passed
echasnovski added a commit that referenced this pull request Sep 9, 2024
@echasnovski
Copy link
Member

This now should be part of main branch. Thanks for finding this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants