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

Declaritive prepl connections #15

Closed
Olical opened this issue Apr 1, 2019 · 12 comments
Closed

Declaritive prepl connections #15

Olical opened this issue Apr 1, 2019 · 12 comments
Assignees
Labels
breaking A breaking change, beware! enhancement New feature or request
Milestone

Comments

@Olical
Copy link
Owner

Olical commented Apr 1, 2019

A thought from today: When adding a connection, it shouldn't matter if you can't connect yet, when you eval it'll try to connect again. If you reboot your REPL it should disconnect and then reconnect when you eval.

My point being: The connection pool should contain instructions for connections then ensure those connections as and when they're required. This allows you to attempt things even when the REPL isn't up yet, or restart the REPL without having to reconnect.

Of course, if you eval and it can't connect it'll log an error, but the next eval will try again. This makes connections essentially declarative!

@Olical Olical added the enhancement New feature or request label Apr 1, 2019
@Olical Olical added this to the v1.0.0 milestone Apr 1, 2019
@Olical Olical self-assigned this Apr 1, 2019
@daveyarwood
Copy link
Contributor

This sounds like a great idea to me. I suspect it would make the experience feel lighter weight, as the REPL might have more time to start in the background while one types out one's first statement to eval.

@mynomoto
Copy link

mynomoto commented May 2, 2019

This sounds good but I think that some way to configure prepl ports and tags per project would be even better. nrepl clients can use dot files generated by repls to know to which port to connect without user input. Since the prepl is created via a config on deps.edn Conjure could check the config on some usual places or even a .conjure file the project to need less user input.

@Olical
Copy link
Owner Author

Olical commented May 3, 2019

I'd have to nail down the mechanics for how it'd work. Like should it keep retrying the same eval if it fails or should it throw it away and log it... maybe stay silent and just notify you of connections and disconnections... so yeah, lots of questions!

As long as the general idea of describing what you'd like to be connected to and then letting Conjure decide when to actually connect doesn't sound silly. A .conjure.edn file sounds really interesting too, I like the idea of that.

Right now I store my custom bindings in .lvimrc and use the local vimrc plugin to load it in each project. I usually bind ,rc to connect to my REPL. If I have more than one it's ,rca (local API) or ,rcs (staging) for example.

@mynomoto
Copy link

mynomoto commented May 3, 2019

TIL about local_vimrc. Thanks!

@Olical Olical added the breaking A breaking change, beware! label May 21, 2019
@Olical
Copy link
Owner Author

Olical commented May 21, 2019

Current thinking is .conjure.edn that gets read from your current dir all the way up to home (I think...), merging all the way. It will have the same syntax as the ConjureAdd command right now.

Then on eval Conjure will attempt to connect to all of your described REPLs, if you bring one up or shut one down it will attach and detach where required. So there will be no commands to connect or disconnect, it'll just happen if it's there. There will be one command that reloads the config and reconnects to everything though just in case you need to change some config or reconnect to a dead prepl.

If there's no config, nothing matches or nothing will connect then it'll prompt you to hit a key mapping or command to self prepl into Conjure's own JVM. So:

  1. Walk up the dirs on boot merging config together.
  2. On eval try to connect to things that aren't connected yet.
  3. Pick the prepls to eval in as normal.
  4. If nothing matches, warn and prompt self prepl.

@daveyarwood
Copy link
Contributor

That sounds great! I like the idea of warning/prompting about doing the self prepl. I could see it being confusing otherwise, like if you think you're connecting to a project REPL but you're actually not.

@Olical
Copy link
Owner Author

Olical commented Jun 9, 2019

Will be released in v0.18.0. Check out the develop branch if you want to try it out or adapt to the breaking changes early! Hint: No more ConjureAdd, ConjureRemove or ConjureRemoveAll. Added .conjure.edn and ConjureUp.

@dharrigan
Copy link
Contributor

dharrigan commented Jun 9, 2019

Hi,

Firstly, great work on the declaritive work. Here are my few comments:

I'm not a fan of having "default" mappings, such as dev, jvm or node. Personally, I call my local development, well, local, since dev is often the first environment (after your local machine) that stuff is tested out on (i.e., dev -> uat -> production, something like that). I'm not saying to introduce a local configuration, but rather, just remove the assumed default lookups altogether. If no .conjure.edn can be found anywhere, don't do anything - i.e., maintain existing behaviour - let the user decide how to connect - as before.

In relation to this, having the conjure log pop up saying it can't connect is noise and distracting. If it can't find a .conjure.edn anywhere, don't pop up anything. If it does find a .conjure.edn, then don't pop up anything (as I think, the principle of only report on exceptions (that are configured) is good)

I don't want the log to tell me that it can't connect to :dev, :jvm or :node since those things will never be a part of my setup (which means, every time I go into vim, I have to close the log window - distracting).

So, on first test, I can't see to get what I expect.

In my $PROJECT folder, I have a .conjure.edn with this setup:

{:conns
 {:local
  {:port 55555}}}

i.e., only one configuration. I would have expected that if there is one (and only one) mapping, then ConjureUp would try to automatically use that one (typing in ConjureStatus shows this

; conjure/out | Welcome to Conjure!
; conjure/out | Skipping :dev - can't connect
; conjure/out | Skipping :jvm - can't connect
; conjure/out | Skipping :node - can't connect
; conjure/out | 0 connections

I would have expected, since there is only one .conjure.edn, then it would have used that as the default for this project and automatically try to connect to the prepl (and I do have a prepl running on port 55555).

Hope this helps as some initial feedback.

-=david=-

@Olical
Copy link
Owner Author

Olical commented Jun 9, 2019

Oh, haha, this is a bug 😆 it's reading the config from Conjure's own directory, not from your project directory as intended, hang on, I'll fix it 😅

@Olical
Copy link
Owner Author

Olical commented Jun 9, 2019

Fixed in develop by asking Neovim for the current working directory when fetching config. I was using Conjure's CWD before which is actually the source folder. Oops 😨

Hopefully that works well for you now!

@dharrigan
Copy link
Contributor

Yes, thank you.

@Olical
Copy link
Owner Author

Olical commented Jun 10, 2019

Released in v0.18.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking A breaking change, beware! enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants