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

Docstrings do not show up in babashka files unless bb.edn is also present #1968

Open
holyjak opened this issue Nov 18, 2022 · 7 comments
Open

Comments

@holyjak
Copy link
Contributor

holyjak commented Nov 18, 2022

When using Calva to edit a babashka script I do not get docstrings in the on-mouse-over popups. The reasons turns out to be that clojure-lsp requires a bb.edn file (which can be as simple as {}) in the root project directory to understand that it is a babashka project and be able to offer documentation. This despite the fact that a jacked-in, selecting a babashka repl so that at least Clava knew this was a bb project.

@skylize
Copy link
Contributor

skylize commented Nov 18, 2022

This is caused by failing to meet the requirements of Clojure LSP. Even though Babashka only needs a bb.edn file for certain features to work, LSP always needs a project config file to know how to build the classpath for the project.

As per discussion in https://clojurians.slack.com/archives/CBE668G4R/p1668764172332819, a proposed way to solve this is bit of trickery by Calva to coerce Clojure LSP into building the correct classpath:

  • When jacking into a bb repl, check for existence of bb.edn.
    • If exists
      • continue as normal.
    • If missing
      • Write a minimal bb.edn to the project root.
      • Restart Clojure LSP
      • Delete the Calva-created bb.edn.

@PEZ
Copy link
Collaborator

PEZ commented Nov 18, 2022

Seems like a nice service we can provide to create that file.

We should probably also fix so that the REPL can help with docstrings and stuff in Babashka projects.

@bpringe
Copy link
Member

bpringe commented Nov 20, 2022

Another option is rather than deleting the created bb.edn after restart clojure-lsp, resulting in the creation + deletion each time the project is opened in Calva, we could add a comment in the created file that says Calva created it so that clojure-lsp features work.

Then it's up to the user if they want to delete it, and if they do the same process will occur next time, but if they don't, there won't be a need for creating and deleting the file + restarting clojure-lsp next time the project is opened.

The only issue I see with the above is that if someone really doesn't want the file in their project, they'll have to delete it each time the project is opened (or .gitignore it if they just don't want to commit it), but I don't really see why someone would want to do that.

@PEZ
Copy link
Collaborator

PEZ commented Nov 21, 2022

Makes total sense leaving the file there with a comment like that.

@PEZ
Copy link
Collaborator

PEZ commented Nov 21, 2022

Another option is to only inform the user about the situation. If Calva doesn't find the file, it informs:

”If this is a Babashka project clojure-lsp needs it to have a bb.edn file. It can be as simple as containing only {}. To get full clojure-lsp service, create this file and restart the clojure-lsp server.”

And two buttons: Don't show again, calva.io/clojure.lsp

Less magic, less things can go wrong. The user remains in full control.

@skylize
Copy link
Contributor

skylize commented Nov 21, 2022

The use case here is non-Clojure related Babashka scripting.

If you have a Clojure project, with Bb helping out, the sensible thing is to add a bb.edn. You will proabably end up using it at some point to configure something. Same if, for some reason, you decide to build a project only in Babashka.

But the original intent of Bababashka is to replace shell scripts. And it would seem really weird if every Bash script also needed some arbitrary additional file in the same directory that doesn't do anything.

In fact Bb does not need this additional file. Only LSP does. And it only needs it momentarily, as a reference point while starting up. Seems to me that creating a temporary file here is mostly equivalent to using temporary environment variable to set a path.

PROJECT_ROOT=/path/to/project clojure-lsp

If we decide against create->delete, and opt for only create, then I agree with @PEZ that we should be up front about it.

@bpringe
Copy link
Member

bpringe commented Nov 24, 2022

But the original intent of Bababashka is to replace shell scripts. And it would seem really weird if every Bash script also needed some arbitrary additional file in the same directory that doesn't do anything.

Fair point. I think I'd rather we do nothing but inform the user that they should create a bb.edn if they want editor support from clojure-lsp, as @PEZ said. Some users might not even care about having that support anyway. We can leave it to them to decide, but at least make them aware, and that prevents us from needing to add the code for creating and deleting the file and restarting clojure-lsp.

The restarting of clojure-lsp could involve a bit of complexity around making sure the server isn't currently starting when we want to restart it (requiring us to wait for it to be running, then restart) or currently stopped. We can avoid adding potential maintenance by avoiding taking action aside from just notifying the user.

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

No branches or pull requests

4 participants