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

Allow connection to AF_UNIX socket nREPL servers #417

Open
chancerussell opened this issue Feb 16, 2024 · 3 comments · May be fixed by #418
Open

Allow connection to AF_UNIX socket nREPL servers #417

chancerussell opened this issue Feb 16, 2024 · 3 comments · May be fixed by #418

Comments

@chancerussell
Copy link

nREPL has had support for running servers through AF_UNIX-style domain sockets for a while: nrepl/nrepl#270 . It'd be nice if we could connect to those servers with vim-fireplace.

I've messed around and have confirmed that vim-fireplace can talk to nREPL over a domain socket with minimal changes to the Connection class's socket method in pythonx/fireplace.py to instantiate the correct type of socket as appropriate. I’m less sure of how starting such a connection should work.

The main question I have is: how should the user indicate that they wish to connect to a domain socket server?

Options I can think of:

  • Smarten up FireplaceConnect to recognize connection strings that represent filesystem locations. This might be problematic because of ambiguous cases where a given string could represent either a hostname or a filesystem location.
  • Add a new function analogous to FireplaceConnect specifically for connecting to domain sockets. This is a little less convenient but removes the ambiguity.
  • Punt and require users to pass in an unambiguous “I want a domain socket connection” string to FireplaceConnect. When nREPL starts up in socket mode, it prints a message like nREPL server started on socket nrepl+unix:$SOCKET_PATH, so maybe treating nrepl+unix: as a magic prefix would suffice.

I’ve opened #416 as a draft proof of concept. It works, but it isn’t pretty.

Thanks for all your work on vim-fireplace!

@tpope
Copy link
Owner

tpope commented Feb 16, 2024

Smarten up FireplaceConnect to recognize connection strings that represent filesystem locations.

We already support literal filesystem locations. But they're used to specify the path to a directory that contains .nrepl-port:

elseif str !~# '^\d\+$\|:\d\|:[\/][\/]' && filereadable(str . '/.nrepl-port')
let path = fnamemodify(str, ':p:h')
let str = readfile(str . '/.nrepl-port', '', 1)[0]

So all you need to do is disambiguate with a getftype() check and you're golden.

This might be problematic because of ambiguous cases where a given string could represent either a hostname or a filesystem location.

The circumstances where this could happen are extremely narrow: <port>, <host>:<port>, and URLs. And if you do hit one of those, you can use ./ to force a file path.

@chancerussell
Copy link
Author

So all you need to do is disambiguate with a getftype() check and you're golden.

Very cool, I can handle that!

When we hit the "str is pointing to a socket file" case, do you think we should just pass the value as the host arg to the python script, or should we have an explicit (new) arg for socket paths?

@tpope
Copy link
Owner

tpope commented Feb 17, 2024

When we hit the "str is pointing to a socket file" case, do you think we should just pass the value as the host arg to the python script, or should we have an explicit (new) arg for socket paths?

I would change the host arg to dest and drop the port arg. Those parameters are legacy: In fireplace#transport#connect() we always coerce it to a URL. (You might want to move this coercion to fireplace#ConnectCommand().)

Then you can pass the socket path through directly, and main() can look at dest and decide if it looks like a URL or a file path. This is unambiguous.

@chancerussell chancerussell linked a pull request May 21, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants