sanel / monroe

Clojure nREPL client for Emacs
161 stars 21 forks source link

General changes #44

Closed Sasanidas closed 1 year ago

Sasanidas commented 1 year ago

As mention in https://github.com/sanel/monroe/issues/43, I've been playing around with monroe and modifying it so it ca be used for more servers other than Clojure. These are my firsts steps, which are not yet addressing the #43 issue, but I think it can help to improve the package.

The changes so far are:

I've been testing these new changes with two nprel servers:

Both seems to work "good enough", but I'm sure there are still bugs to fix.

technomancy commented 1 year ago

Sometimes people ask me "why use monroe when cider has more features?" and I'm like "monroe is much simpler and easier to understand; for instance, it's just one file instead of a ton of different files"

Moving from "it's all in one file" to "it's multiple files" is kind of a big change, and I don't think it should be done without some discussion. Maybe it's worth it, but personally I'd prefer if it was all in one.

Also, the new version is dramatically longer; what are the shortcomings of the old implementation which motivate the move to a more complex implementation? Not to say we shouldn't do it, but we should be clear about the trade-offs.

sanel commented 1 year ago

Yeah, I'm with @technomancy here. At first, I was confused with the amount of changes. Why new bencode, what were the problems with the previous one? Some monroe general guidelines & philosophy:

  1. Should be simple. KISS.
  2. Single file. No dependency. Support for older Emacs-es.
  3. Must work out of the box. Download a single file and run it.
  4. Keep the previous behavior.
  5. Stable. No "seems to work" :) Either cover all the cases or none (probably the main pain point is not having extensive tests for it, but TBH I'm not sure how to properly test it either).
Sasanidas commented 1 year ago

Thanks for the feedback

Also, the new version is dramatically longer; what are the shortcomings of the old implementation which motivate the move to a more complex implementation? Not to say we shouldn't do it, but we should be clear about the trade-offs.

This new and longer implementation of bencode it's very well documented, It as specific error types for parsing errors and in my opinion even tho it's longer, it easier to debug if something goes wrong.

Some monroe general guidelines & philosophy: Should be simple. KISS. Single file. No dependency. Support for older Emacs-es. Must work out of the box. Download a single file and run it. Keep the previous behavior. Stable. No "seems to work" :) Either cover all the cases or none (probably the main pain point is not having extensive tests for it, but TBH I'm not sure how to properly test it either).

I understand, my idea was to fully support the NREPL protocol but not Clojure specific (which as @technomancy point out I would have to ping the NREPL team to maybe change some things).

I wasn't aware of these constrains which I think are complete reasonable, but not in line for what I had in mid to contribute for this project, sorry for the misunderstanding.

As my intentions are not align with the project, I'll continue with the fork (maybe change the name given that I'll probably diverge even more).

Again, thanks for writing and maintaining monroe, I'll be closing now this merge request and the issue associated with it.

Good luck! :wave:

sanel commented 1 year ago

Thank you @Sasanidas ! If you happen to make any improvements that could be merged in monroe, I'll be more than happy to check them :)