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

Cljs loses its repl connection #2351

Closed
dpsutton opened this issue Jun 26, 2018 · 25 comments
Closed

Cljs loses its repl connection #2351

dpsutton opened this issue Jun 26, 2018 · 25 comments

Comments

@dpsutton
Copy link
Contributor

At work we have two projects running at any particular time: the front end cljs project and the clj backend. I normally don't interact with the frontend repl that much as figwheel reloads it quickly. But I am often querying against the backend, etc, in the backend repl. However, when I do this, the cljs buffers are essentially broken. Their "session" is entirely based off of the last visited repl.

So if i query something or even if point cycles through the clj repl, when i visit the frontend buffers I get:

user-error: ‘cider-find-var’ requires the nREPL op "info". Please, install (or update) cider-nrepl 0.18.0-SNAPSHOT and restart CIDER

I can quickly pull up the cljs repl and then bury the buffer and all is well in the world.

Each of these projects have their own directory structure rooted at a project.clj file. I don't follow the rationale that CIDER now just attempts the last repl buffer rather than remembering some kind of connection details per "project". This example is even simpler because all clj files go to the backend repl and all cljs repls go to the front end.

Am I missing some notion of sessman to link directories to sessions?

@bbatsov
Copy link
Member

bbatsov commented Jun 26, 2018

@vspinu is the expert here, but in my experience it seems that the session is directly mapped to the project root and within the session the last connection of each type is the default one.

@vspinu
Copy link
Contributor

vspinu commented Jun 26, 2018

@dpsutton, just navigate to the relevant sub-directory and link it to the session of your choice (C-c C-s d). See sesman menu or map for what is available.

CIDER now just attempts the last repl buffer rather than remembering some kind of connection details per "project".

Don't you say that both directories are rooted in the same project? I guess something specific to cider could be done here (like specialized context) but I don't see a use pattern from what you have described.

I still have to write proper docs for cider but here is the crux. With sesman you are in control of all your associations at the session level (not to be confused with nrepl-session). Sesman sessions are created with jack-in-xyz or connect-xyz and repls (aka connections) are added to sessions with connect-sibling-xyz. Sessions are linked to contexts (project, directory or buffer). By default they are linked to the least specific context (project, or directory if no project exists). In your case two sessions are linked to project and they are disambiguated by the most recent used session. You can disable automatic diambiguation but then you will be asked for session on every single operation.

@dpsutton
Copy link
Contributor Author

Thanks for the info @vspinu . These two projects are in the same git repo but have separate project.cljs. Not sure if that answers the question

@dpsutton
Copy link
Contributor Author

That seems to be the missing piece I was missing.

@vspinu
Copy link
Contributor

vspinu commented Jun 26, 2018

I see. That's the raw part. This happens because sesman is using project.el for project detection, but clojure-mode has no support for it yet. I finally have some time to work on cider today and tomorrow. Will fix that unless @bbatsov outruns me on it.

@bbatsov
Copy link
Member

bbatsov commented Jun 26, 2018

I actually added support for this earlier today and released a new version of clojure-mode. The problem, however, is that project.el is implemented in a really primitive manner - it just calls a bunch of hook functions until some of them returns a result, which means the one that's first in the list wins. So even if clojure-mode adds its clojure discovery function it might still be called after the vc check that project.el uses by default.

@bbatsov
Copy link
Member

bbatsov commented Jun 26, 2018

@vspinu
Copy link
Contributor

vspinu commented Jun 27, 2018

The problem, however, is that project.el is implemented in a really primitive manner - it just calls a bunch of hook functions until some of them returns a result, which means the one that's first in the list wins.

Hmm. This might be a show stopper actually. With VC it's unlikely to happen because it's part of project-find-functions by default and add-hook adds in front, but if someone else inserts a hook in front it will overwrite Clojure root.

In principle adding it as a local hook would solve the problem, but that would be quite inconvenient in CIDER because it would have to add the hook in every single type of buffer it creates. And it would still leave the possibility that some of session commands can be called outside of default CIDER modes :/

@dgutov any thoughts of why project.el is relying on run-hook-with-args-until-success and is not picking the innermost directory instead? Could (should?) something be done about it?

@bbatsov
Copy link
Member

bbatsov commented Jun 27, 2018 via email

@dgutov
Copy link
Contributor

dgutov commented Jun 27, 2018

The problem, however, is that project.el is implemented in a really primitive manner - it just calls a bunch of hook functions until some of them returns a result, which means the one that's first in the list wins.

Is that a practical concern, or a theoretical one?

So even if clojure-mode adds its clojure discovery function it might still be called after the vc check that project.el uses by default.

Like Vitalie said, this situation is very easy to avoid.

any thoughts of why project.el is relying on run-hook-with-args-until-success and is not picking the innermost directory instead? Could (should?) something be done about it?

First of all, guys, you are 3 years late to this discussion. And it's not like you haven't been invited.

Second, I still think it's the better choice. Of course, the deciding factor was the simplicity of the implementation (for project-current), since nobody really tried to make the argument you're making now, before. And it offloads the choice of the algorithm of picking the project root to the project backends. Which is a good thing because we (in the Emacs core) don't have to predict these requirements in advance.

Now, you're worried about some other backend coming first. To do that, it would have to return non-nil, which means it declares itself responsible for the current file. And yeah, if it steps on your toes, taking responsibility for files it shouldn't, it can lead to conflicts. But if the user installs two different project-backend packages that try to compete, it can lead to conflicts anyway, no matter which strategy project-current chooses, whether it's "first one wins", or "innermost wins".

Further, how would the "innermost wins" really help? Suppose some project backend comes first in project-find-functions, and it declares a subdirectory of your Clojure project as "his". You're still going to have a problem, only limited to that specific subdirectory. Unless, of course, your project backend and that other one are, by stroke of luck, highly compatible somehow. But it's not really possible to rely on that, unless both backends' development is inter-coordinated. And if that happens, you might as well merge the project backends together, and use specialized logic as necessary.

In short, I wouldn't worry about having to come first, unless it becomes a real problem. And then it will likely be a political one, not technological. Though maybe it will require some extra documentation.

@bbatsov
Copy link
Member

bbatsov commented Jun 27, 2018

Is that a practical concern, or a theoretical one?

It's theoretical until it becomes practical. ;-) Anyways, for now I'll just insert the clojure-mode stuff first and see how this goes.

Like Vitalie said, this situation is very easy to avoid.

I certainly don't want to litter the entire CIDER codebase with setting local hooks, so I'd argue that "easy" here is not the same as a "good solution".

First of all, guys, you are 3 years late to this discussion. And it's not like you haven't been invited.

Fair enough. I was busy with other things and I generally have little interest in the project, as I believe I mentioned then as well. But I don't want to go into this now. If it wasn't for @vspinu's desire to use project.el in sesman I'd probably would have never taken a closer look at the project.

Second, I still think it's the better choice. Of course, the deciding factor was the simplicity of the implementation (for project-current), since nobody really tried to make the argument you're making now, before. And it offloads the choice of the algorithm of picking the project root to the project backends. Which is a good thing because we (in the Emacs core) don't have to predict these requirements in advance.

I see your point, but I've also seen while working on Projectile that you often end up having different potential projects in the same dir structure if there's no way to evaluate all the project options that were found you can't pick the project the user actually wants to work on.

In short, I wouldn't worry about having to come first, unless it becomes a real problem. And then it will likely be a political one, not technological. Though maybe it will require some extra documentation.

I might write it myself. At least a few integration notes in project.el.

@dgutov
Copy link
Contributor

dgutov commented Jun 27, 2018

I certainly don't want to litter the entire CIDER codebase with setting local hooks, so I'd argue that "easy" here is not the same as a "good solution".

No need for local hooks here. You just use add-hook without adding the APPEND argument.

I've also seen while working on Projectile that you often end up having different potential projects in the same dir structure if there's no way to evaluate all the project options that were found you can't pick the project the user actually wants to work on.

Multiple Projectile-based projects? There should be just one element in project-find-functions corresponding to Projectile.

I might write it myself. At least a few integration notes in project.el.

Thanks in advance.

@vspinu
Copy link
Contributor

vspinu commented Jun 27, 2018

@dgutov

And it offloads the choice of the algorithm of picking the project root to the project backends.

I was supporting project.el here and elsewhere in the hope of having uniform project lookup, and I am a bit surprised that such a basic task is "offloaded". I think that project.el could easely provide a few extra helpers like package-innermost-root and package-outermost-root.

Further, how would the "innermost wins" really help? Suppose some project backend comes first in project-find-functions, and it declares a subdirectory of your Clojure project as "his". You're still going to have a problem, only limited to that specific subdirectory.

This is a valid point, but the nature of the "problem" is specific to the task at hand. A tool like CIDER won't be using project.el because it is only interested in clojure projects. A generic tool that aims to provide project specific functionality (sesman in this case) is much more likely to use project.el. And if it does so, it will probably use innermost and less likely outermost root.

The two "problems" discussed here are of a different nature. One is the problem with relying on the order of hooks in project-find-functions, which is essentially random. The other one is relying on the nested directories, which is most of the time under the user's control. A user could create partitions of functionality on the fly with simple .Projectile or maybe .dir-locals markers which generic tools could pick up. project.el itself aims at providing an implementation of project-local variables, and (I am guessing here) it should use innermost definition for those.

@bbatsov

I think it’d be best to just add some simple project search registration to sesman for now.

I thought of adding a generic which would fall back on project-current but that would tie the root lookup with the backend and would make user-driven fragmentation of the project directory very hard. For instance one might want to automatically associate cljs-figwheel repl in one sub-tree and plain clj repl to another. In my work with R I often want to start localized sessions in test/ or data/ directories regardless of the project root. But well, this idea is still in a half backed state and might not be as fruitful as I imagine it to be. It's quite possible that two notions of project root might be required, one for the backend and one for the user, in which case a generic for the backend should be just fine.

@dpsutton
Copy link
Contributor Author

Thanks everyone for thinking this over!

@dgutov
Copy link
Contributor

dgutov commented Jun 28, 2018

I was supporting project.el here and elsewhere in the hope of having uniform project lookup, and I am a bit surprised that such a basic task is "offloaded"

What is offloaded, is the understanding of what a project is. Which can vary between projects.

I think that project.el could easely provide a few extra helpers like package-innermost-root and package-outermost-root.

And then what? The client will have to pick which project it wants (innermost, outermost, etc)? That doesn't sound like "uniform project lookup" to me. Not when different commands, in the same buffer, might end up acting on different projects.

Not to mention that a project can have more than one root, which complicates the situation.

And if it does so, it will probably use innermost and less likely outermost root.

If even you are not 100% sure now, then, if I understand the end of your message correctly, "two notions of project root ..., one for the backend and one for the user" might be the way to go.

@dgutov
Copy link
Contributor

dgutov commented Jun 28, 2018

One is the problem with relying on the order of hooks in project-find-functions, which is essentially random.

The order of elements in completion-at-point-functions is also "essentially random", and yet we usually get acceptable results from it.

A user could create partitions of functionality on the fly with simple .Projectile or maybe .dir-locals markers which generic tools could pick up.

This implies that whatever project backends are installed, will honor these properly.

project.el itself aims at providing an implementation of project-local variables, and (I am guessing here) it should use innermost definition for those.

It will use the variables assigned to the current project, whichever way that happens. Probably using a file inside a project root. But just as well it should be some alist somewhere in the user's init script, or the Custom file.

@vspinu
Copy link
Contributor

vspinu commented Jun 28, 2018

And then what? The client will have to pick which project it wants (innermost, outermost, etc)?

Yes. It depends on the client's intent. For a project-wide reg-exp replacement the outermost root might be appropriate. For associations to which repl the code is sent, the inner root is probably more appropriate. For project-local variables, inner-root settings should have precedence etc.

The order of elements in completion-at-point-functions is also "essentially random", and yet we usually get acceptable results from it.

Well, this is precisely why I have to place huge composite backends in front of my company-backends. It's just not ideal. Linear pipelines are great, but nil-punning pipelines are just too primitive for most tasks, be it completion, xref or root lookup.

A user could create partitions of functionality on the fly with simple .Projectile or maybe .dir-locals markers which generic tools could pick up.

This implies that whatever project backends are installed, will honor these properly.

If the backend is reached in the pipeline ;)

"two notions of project root ..., one for the backend and one for the user" might be the way to go.

Indeed. I think the best solution is to make multiple implicit associations simultaneously. One for proper clojure root and others for all intermediate projects between clojure root and current file. This way user could re-associated repls to any of the intermediate projects.

But for the above, I would need an utility in project.el along the lines of project-roots-between-dirs which well ... is not there :/

Not to mention that a project can have more than one root, which complicates the situation.

"Complicates" is right word. A cleaner and a more intuitive API might have been to restrict the project-current to be a single root and just have an extra helper (project-external-roots) to compute other roots on demand. Right now all project.el consumers have to deal with the possibility of multiple roots. I have no intention to deal with that in sesman; will just pick the first one and hope that it will be the "right" one.

@dgutov
Copy link
Contributor

dgutov commented Jun 29, 2018

It depends on the client's intent.

So now intent enters the picture? I hope you understand that then we'd have to classify and document the intents. Which is a complication for the API, at the very least.

To get that in, I think you will need to show how different kinds of commands, not just belonging to cider or sesman, would make use of different intents.

Well, this is precisely why I have to place huge composite backends in front of my company-backends.

This is interesting, but you're clearly an outlier. And, as you can see, the approach does work; it's just, in certain cases, the elements end up being complex.

If the backend is reached in the pipeline ;)

Yes, but you're missing the point: all the backends in the list would have to obey a certain kind of new principle. It's much easier to organize that if there's just one backend, and you are controlling it.

But... after that, I'm not exactly sure that project.el is something that would help you much. If you are controlling a project backend, and if you want to interact with only it in sesman, there's no need for a project-agnostic API to facilitate that.

But for the above, I would need an utility in project.el along the lines of project-roots-between-dirs which well ... is not there :/

I don't think it's particularly difficult to write, though. It should amount to 4-5 lines.

"Complicates" is right word. A cleaner and a more intuitive API might have been to restrict the project-current to be a single root and just have an extra helper (project-external-roots) to compute other roots on demand.

External roots hold a somewhat different meaning: system includes, libraries, and similar directories. Dependencies, so to speak.

Right now all project.el consumers have to deal with the possibility of multiple roots. I have no intention to deal with that in sesman; will just pick the first one and hope that it will be the "right" one.

That possibility has been added at the request of Stephen Leake, the main author of Ada development tools for Emacs. And in that world, projects apparently do span several disjointed directories.

Looking at an external example, though, projects in Atom can also span multiple directories: https://atom.io/docs/api/v1.28.0/Project

Personally speaking, I also find this possibility unnecessary, but to remove it, we'd have to propose some adaptation for Ada projects.

@vspinu
Copy link
Contributor

vspinu commented Jun 29, 2018

It depends on the client's intent.

I hope you understand that then we'd have to classify and document the
intents. Which is a complication for the API, at the very least.

What I propose is to introduce a few extra helpers to manipulate root retrieval. I don't see why this is such a complication of the API. If it doesn't align with your vision of project.el it could be a specialized package "project-root.el" or something. But at least I hope you would agree that it's an essential functionality which should be built in.

To get that in, I think you will need to show how different kinds of commands,
not just belonging to cider or sesman, would make use of different intents.

Or I have to show that the current implementation is flawed. If it doesn't make a difference for project.el itself which root is selected then deciding between piking by default the first non-nil or picking innermost should be non-controversial.

Well, this is precisely why I have to place huge composite backends in front of my company-backends.

the approach does work

It does, but it's not ideal. The assumption in all those nil-terminating pipelines is that backend knows where to stop, but that's just wrong. Backends don't see the big picture and shouldn't be tasked with stopping the pipeline.

If the backend is reached in the pipeline ;)

Yes, but you're missing the point: all the backends in the list would have to
obey a certain kind of new principle.

That's not true for the root example. There is no need for more "principles" than are already there. If each backend retrieve one root which it see fit, each application will retrieve the root or roots which it deems appropriate.

It's much easier to organize that if there's just one backend, and you are
controlling it.

Then I wouldn't need package.el.

But... after that, I'm not exactly sure that project.el is something that would
help you much. If you are controlling a project backend, and if you want to
interact with only it in sesman, there's no need for a project-agnostic API to
facilitate that.

No, I need to be able to operate on non-clojure sub-projects (potentially created by users with simple markers like .Projectile, or maybe .sesman).

Personally speaking, I also find this possibility unnecessary, but to remove
it, we'd have to propose some adaptation for Ada projects.

Removal wouldn't be necessary if project.el could make a guaranty that the first root in the list is "the root" - the root which is directly accessible from the current buffer.

@vspinu
Copy link
Contributor

vspinu commented Jun 29, 2018

To get that in, I think you will need to show how different kinds of commands, not just belonging to cider or sesman, would make use of different intents.

Maybe @bbatsov could shed some light here on what's the use of innermost/outermost roots and range of roots in the wild. Projectile has a variety of search mechanisms for roots.

@dgutov
Copy link
Contributor

dgutov commented Jun 29, 2018

I don't see why this is such a complication of the API.

You seem to be dodging certain parts of my answers. Let's try again.

If you really serious about this discussion, though, please file a bug in the Emacs bug tracker, or at least write to emacs-devel. GitHub comments are nice and all, but it's not good when important discussions related to Emacs are led on a closed platform.

If it doesn't align with your vision of project.el it could be a specialized package "project-root.el" or something.

I have tried hard not to make it "my vision". But indeed, it does not fit the concept as I see it now.

Like I said, this functionality will take like 4 lines to write, so no, I wouldn't consider it essential, even if I agreed that it's a good idea in certain context. Don't take that to mean that I'm 100% against including it, but it's certainly doesn't look like a priority.

If it doesn't make a difference for project.el itself which root is selected then deciding between piking by default the first non-nil or picking innermost should be non-controversial.

Right. "If".

The assumption in all those nil-terminating pipelines is that backend knows where to stop, but that's just wrong. Backends don't see the big picture and shouldn't be tasked with stopping the pipeline.

I think they are best suited to "see the big picture". But if not the backends, then who?

Each individual command? Are they all supposed to examine all backend candidates and make the decision somehow? There goes the "uniform project lookup". I don't think that different commands having different notion of the current project is a good idea. If you think so, please go ahead and back that concept up with something. In particular: how should a random command choose which project picking strategy to use?

@dgutov
Copy link
Contributor

dgutov commented Jun 29, 2018

No, I need to be able to operate on non-clojure sub-projects (potentially created by users with simple markers like .Projectile, or maybe .sesman).

Do non-clojure projects include certain files in sub-sub-directories, like Makefile, or project.json, or build.xml, and stuff?

Imagine that the user has a package installed that adds a backend which recognizes one of these as a root marker. Then you will have a subdirectory inside of one of those "non-clojure sub-projecs" (as you understand the notion) which belongs to an alien kind of project, one you didn't anticipate. Am I wrong? Is this kind of situation absolutely impossible?

Then I wouldn't need package.el.

It might indeed turn out that your needs are different here.

Removal wouldn't be necessary if project.el could make a guaranty that the first root in the list is "the root" - the root which is directly accessible from the current buffer.

You mean, if you can always ignore the rest? That doesn't sound like a great API, or use of it.

Anyway, you can always find "the root" with cl-find and file-in-directory-p, if I understand your meaning of "directly accessible" correctly.

@vspinu
Copy link
Contributor

vspinu commented Jun 29, 2018

how should a random command choose which project picking strategy to use?

A "random command" will rely on the default. Only those tools which do have solid reasons will deviate.

Am I wrong? Is this kind of situation absolutely impossible?

You are right, it's not optimal. But the question here is if it's better than the current one. You can apply your hypothetical construct to the current implementation and get a random root. Exiting.

which belongs to an alien kind of project, one you didn't anticipate

That's completely fine for my use case because I don't "anticipate" a specific kind of project. If a tool defines a marker for project it has reasons to do so, if a user uses such a tool, he or she has reasons to do so, thus sesman will propose those sub-projects as part of its completion, etc.

You mean, if you can always ignore the rest? That doesn't sound like a great API, or use of it.

Fair enough. I have to agree, being accessible from current context is probably not "special" enough to make a dedicated API.

@dgutov
Copy link
Contributor

dgutov commented Jul 4, 2018

You can apply your hypothetical construct to the current implementation and get a random root. Exiting.

But the backends that go first have a chance to take control. And if less-clever backends go later, the crisis is averted.

And the order of the elements in project-find-functions is, to an extent, controllable. The user can also change the order in which they enable the relevant minor modes, or change the value of this list in a major mode hook. We could also introduce explicit priorities, but the core devs seem to shy away from this concept (to avoid an arms race, they say).

When there's no ordering in backends, however, there is no chance for a smarter one to take control.

That's completely fine for my use case because I don't "anticipate" a specific kind of project. If a tool defines a marker for project it has reasons to do so, if a user uses such a tool, he or she has reasons to do so, thus sesman will propose those sub-projects as part of its completion, etc.

Ok, then I don't understand your use case very well. But I think it can work automatically too, as long as the "inner", more specific projects come first in the list. If such an ordering can exist, of course.

@bbatsov
Copy link
Member

bbatsov commented Jul 14, 2018

Guess we can close this one. Hopefully things are working ok for @dpsutton with the latest clojure-mode.

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

No branches or pull requests

4 participants