Add CIDER log middleware

[r0man]

This PR adds the CIDER log middleware, now with logjam extracted into its own repository.

The initial PR was this one:
#773

And this for the CIDER side:
clojure-emacs/cider#3352

Before submitting a PR make sure the following things have been done:

• The commits are consistent with our contribution guidelines
• You've added tests to cover your change(s)
• All tests are passing
• The new code is not generating reflection warnings
• You've updated the README (if adding/changing middleware)

Thanks!

[r0man]

@vemv @bbatsov Do we want to inline the logjam dependency or not?

I gave it a try, but it looks like running the tests with the rewritten logjam library fail, because they reference the logjam library and not the re-written one. I also tried adding the non-rewritten logjam library to the test profile, but that also fails.

If we want to inline I can take a look at somehow getting mr anderson to work with it, or get rid of the references in the tests (I'm using framework/log there to log test messages without having to deal with the specific implementation).

[vemv]

LGTM - impresive work!

Some superficial suggestions.

[vemv]

(non-blocking)

you'd have to use this pattern in logjam https://github.com/metosin/malli/blob/master/resources/clj-kondo/clj-kondo.exports/metosin/malli/config.edn

[r0man]

Do you mean to export this symbol, so it is picked up by clojure lsp in other projects?

I think we don't need this here. It's a macro defined and used only in the cider-nrepl tests. It it not used by other libraries. But maybe you mean something else?

[vemv]

Do you mean to export this symbol, so it is picked up by clojure lsp in other projects?

Sure, if logjam includes that file, any consumers (including cider-nrepl) will pick it up. It's DRYer.

[vemv]

@vemv @bbatsov Do we want to inline the logjam dependency or not?

Depends on who you ask. 😄

I'd prefer so. But from memory, it can be tricky for it to work in tests. We'd have to look at what refactor-nrepl or some other example does.

But that can be left for later.

[bbatsov]

I'm fine with not inlining the dep - I don't expect any runtime conflicts with it.

Btw, how are the tests passing on Clojure 1.8? I thought logjam required 1.9.

[vemv]

I'm fine with not inlining the dep - I don't expect any runtime conflicts with it.

Yeah especially given that it brings no transitive deps in.

Maybe just add:

[mx.cider/logjam "0.1.1"] ;; not inlined because it's internal CIDER tooling with no transitive dependencies (please verify this remains the case whenever you update this dep)

[r0man]

I'm fine with not inlining the dep - I don't expect any runtime conflicts with it.

Btw, how are the tests passing on Clojure 1.8? I thought logjam required 1.9.

I was also wondering when I initially opened the PR. Now that we have logjam in a separate repo, and we don't ship the specs nor reference them, it's not a hard requirement for depending on it. It's only needed for tests.

[r0man]

@vemv Thanks for the review. I hope I addressed your comments.

[bbatsov]

I was also wondering when I initially opened the PR. Now that we have logjam in a separate repo, and we don't ship the specs nor reference them, it's not a hard requirement for depending on it. It's only needed for tests.

Ah, even better! This means that the new functionality can land in the next CIDER release!

[vemv]

I hope I addressed your comments.

You sure did - kudos!