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

Bridge unix sockets and named pipes #1828

Merged
merged 14 commits into from Apr 8, 2023
Merged

Conversation

Kethku
Copy link
Member

@Kethku Kethku commented Apr 7, 2023

Fixes up #1591 on windows

This PR adds support for connecting to Neovim instances over Unix domain sockets and named pipes. It resolves #1412.

Named pipes are a Windows feature, and since I don't run Windows, I haven't been able to test that part.

I have added --server as a new command line argument that covers all connections to existing Neovim instances, because

  1. It's a reasonably unambiguous name
  2. It's inappropriate to use --remote-tcp as the official way to connect to Unix sockets and named pipes
  3. I don't like having remote as part of the name
    • The feature also allows local connections, which are much more useful
    • Unix sockets and named pipes are local by definition as far as I know
    • It encourages users to connect directly to a remote instance over TCP, which is something they probably shouldn't do
    • One could argue that remote is appropriate because in this context it means non-embedded rather than physically remote. If so, I would still recommend that we avoid the term due to the ambiguity
  4. It mimics Neovim's --server argument, so it should be familiar for Neovim users
    • The address is parsed in the same way as in Neovim. See :h serverstart

What kind of change does this PR introduce?

  • Feature
  • Refactor

Did this PR introduce a breaking change?

A breaking change includes anything that breaks backwards compatibility either at compile or run time.

  • No. The --remote-tcp argument is still supported as an alias for --server

@Kethku Kethku merged commit 31f9ca5 into main Apr 8, 2023
16 checks passed
@Kethku Kethku mentioned this pull request Apr 8, 2023
falcucci pushed a commit to falcucci/neovide that referenced this pull request Apr 19, 2023
* Use tokio compat_write as TxWrapper.

This makes TxWrapper a misnomer. I'll rename it in a follow-up commit to
facilitate easier reviewing.

* Rename TxWrapper -> NeovimWriter.

* Bridge: Add support for Unix sockets and named pipes.

* Document the `--server` flag.

* Add --server to command line reference.

* connection.rs: Clean up parameter and return types.

* Remove unused file.

* Bridge: Introduce NeovimSession and NeovimInstance abstractions.

Hopefully this makes the connection logic easier to understand.

* Bridge: Rename connection.rs -> session.rs.

* Replace core::result::Result with std::result::Result.

* Remove await on non future for windows builds

* Fixup named pipe functionality and update feature.md docs

* Fix clippy warning

* Fix test build error

---------

Co-authored-by: khjorth <13965958+khjorth@users.noreply.github.com>
@Kethku Kethku deleted the bridge-unix-sockets-and-named-pipes branch May 11, 2024 23:42
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

Successfully merging this pull request may close these issues.

Proper support for unix domain socket based remote connections
2 participants