jack-in support for the Basilisp Clojure dialect in Python

ikappaki commented 1 month ago

Hi,

can you please consider merging this patch to support Basilisp jack-in in CIDER. it resolves #3682.

I have added added an integration test and updated the documentation accordingly.

Currently the integration test currently fails on MS-Windows and has been temporarily disabled. This issue is addressed by https://github.com/basilisp-lang/basilisp/issues/865 but the fix has not been released yet in PyPi. The problem is related flushing the stdout after a println. It only fails in CI, but works fine when invoked from a user's session.

Additionally, it requires clojure-mode v5.19 to support recognizing Basilisp's .lpy file extension

Thanks

Before submitting the PR make sure the following things have been done (and denote this by checking the relevant checkboxes):

[x] The commits are consistent with our contribution guidelines
[x] You've added tests (if possible) to cover your change(s)
[x] All tests are passing (eldev test)
[ ] All code passes the linter (eldev lint) which is based on elisp-lint and includes
- byte-compilation, checkdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.
[x] You've updated the changelog (if adding/changing user-visible functionality)
[x] You've updated the user manual (if adding/changing user-visible functionality)

Thanks!

If you're just starting out to hack on CIDER you might find this section of its manual extremely useful.

bbatsov commented 1 month ago

The PR looks good to me overall, sans my small remarks.

Does Basilisp implement the nREPL protocol fully? I'm wondering if there are some caveats that need to be documented if some ops are not implemented.

ikappaki commented 1 month ago

The PR looks good to me overall, sans my small remarks.

removed obsolete option.

Does Basilisp implement the nREPL protocol fully? I'm wondering if there are some caveats that need to be documented if some ops are not implemented.

The Basilisp nREPL server currently implements the followings ops: eval, describe, info, eldoc, clone, close, load-file and complete, which cover the fundamental ops in my opinion. If you believe there are any caveats that need to be documented, please let me know.

ikappaki commented 1 month ago

I've just realized that jacking in on MS-Windows won't work even in a user session (I left a flush in my modified Basilisp package while testing), contrary to what I mentioned earlier. I think it's better to wait until the new Basilisp release with the fix is out before merging ...

Currently the integration test currently fails on MS-Windows and has been temporarily disabled. This issue is addressed by https://github.com/basilisp-lang/basilisp/issues/865 but the fix has not been released yet in PyPi. The problem is related flushing the stdout after a println. It only fails in CI, but works fine when invoked from a user's session.

bbatsov commented 1 month ago

The Basilisp nREPL server currently implements the followings ops: eval, describe, info, eldoc, clone, close, load-file and complete, which cover the fundamental ops in my opinion. If you believe there are any caveats that need to be documented, please let me know.

I'm guessing you mean completions and lookup, not complete and info, right? Not a big difference for CIDER, but the former are nREPL ops and the latter are cider-nrepl ops, which served as the basis for the nREPL ops. See https://metaredux.com/posts/2020/06/15/nrepl-0-8-evolving-the-protocol.html for details.

ikappaki commented 1 month ago

Hi @bbatsov,

I've checked in an update referencing the latest basilisp version with the recently released fix (thanks @chrisrink10). All tests now pass.

The Basilisp nREPL server currently implements the followings ops: eval, describe, info, eldoc, clone, close, load-file and complete, which cover the fundamental ops in my opinion. If you believe there are any caveats that need to be documented, please let me know.

I'm guessing you mean completions and lookup, not complete and info, right? Not a big difference for CIDER, but the former are nREPL ops and the latter are cider-nrepl ops, which served as the basis for the nREPL ops. See https://metaredux.com/posts/2020/06/15/nrepl-0-8-evolving-the-protocol.html for details.

I failed to mention earlier that the nREPL server is a port of the nbb nREPL server. I've kept the same ops, minus the macroexpand (which I couldn't find a use for at the time) and classpath which I plan to follow up at some point.

I haven't looked at the standard supported ops, but I'm reluctant of remove or renaming anything at this stage in fear I might break Calva support. I'll clean it up in the next basilisp nREPL update.

By the way, the complete keyword does appear to be used by CIDER (not sure if you meant it is not).

thanks

(-->
  id         "3"
  op         "describe"
  session    "b8a83d6d-84ee-4ab3-a1c0-92cd548634cf"
)
(<--
  id         "3"
  session    "b8a83d6d-84ee-4ab3-a1c0-92cd548634cf"
  ops        (dict
               clone     (dict)
               close     (dict)
               complete  (dict)
               describe  (dict)
               eldoc     (dict)
               eval      (dict)
               info      (dict)
               load-file (dict))
  status     ("done")
  versions   (dict
               basilisp (dict
                          incremental    "1"
                          major          "0"
                          minor          "."
                          version-string "0...1...0.b.2")
               python   (dict
                          incremental    "1"
                          major          "3"
                          minor          "12"
                          version-string "3.12.1 (tags/v3.12.1:2305ca5, Dec  7 2023, 22:03:25) [MSC v.1937 64 bit (AMD64)]"))
)
(-->
  id                        "10"
  op                        "complete"
  session                   "b8a83d6d-84ee-4ab3-a1c0-92cd548634cf"
  context                   "nil"
  enhanced-cljs-completion? "t"
  ns                        "user"
  prefix                    #("printl" 0 1 (fontified t) 1 2 (fontified t) 2 3 (fontified t) 3 4 (fontified t) 4 5 (fontified t) 5 6 (font-lock-multiline t fontified t))
)
(<--
  id          "10"
  session     "b8a83d6d-84ee-4ab3-a1c0-92cd548634cf"
  completions ((dict "candidate" "println" "ns" "basilisp.core" "type" "function")
 (dict "candidate" "println-str" "ns" "basilisp.core" "type" "function"))
  status      ("done")
)
(-->
  id         "11"
  op         "eldoc"
  session    "b8a83d6d-84ee-4ab3-a1c0-92cd548634cf"
  context    "nil"
  ns         "user"
  sym        "println"
)
(<--
  id         "11"
  session    "b8a83d6d-84ee-4ab3-a1c0-92cd548634cf"
  docstring  "Print the arguments to the stream bound to :lpy:var:`*out*` ..."
  eldoc      (nil
 ("x")
 ("x" "&" "args"))

  name       "println"
  ns         "basilisp.core"
  status     ("done")
  status     ("done")
  type       "function"
)

bbatsov commented 1 month ago

By the way, the complete keyword does appear to be used by CIDER (not sure if you meant it is not).

CIDER users both it's own ops (if present) and the standard nREPL ops if they are not. In general I think it's best for most nREPL implementations to stick to the official ops for completions and lookup, as this means they would work with more clients. I'm reasonably sure Calva supports completions and lookup, but I've never used it, so I can't be sure. Anyways, that's not a big deal for CIDER, just a general remark.

I failed to mention earlier that the nREPL server is a port of the nbb nREPL server. I've kept the same ops, minus the macroexpand (which I couldn't find a use for at the time) and classpath which I plan to follow up at some point.

nbb supports some built-in and some additional ops and that's fine. I've meant for a while to rename the CIDER ops to something like cider/complete and cider/info, so it's easier to figure out what's part of the protocol and what's a CIDER extension, but I never got to doing so.

ikappaki commented 1 month ago

Hi @bbatsov

CIDER users both it's own ops (if present) and the standard nREPL ops if they are not. In general I think it's best for most nREPL implementations to stick to the official ops for completions and lookup, as this means they would work with more clients.

Is this the official list of operations supported by the nREPL protocol that servers should adhere to? https://nrepl.org/nrepl/ops.html

I've meant for a while to rename the CIDER ops to something like cider/complete and cider/info, so it's easier to figure out what's part of the protocol and what's a CIDER extension, but I never got to doing so.

That would be very helpful indeed.

clojure-emacs / cider

jack-in support for the Basilisp Clojure dialect in Python #3683