Give a middleware name for pprint so it can be discovered dynamically

[laurentpetit]

For CCW I make use of the describe op to discover available middlewares.

The proposed change would enable pprint to be discovered like any other middleware.

[expez]

@laurentpetit ping, in case you didn't notice the test failure.

[laurentpetit]

@expez Thanks. Fixed.

[expez]

Could you also please squash the commits?

[laurentpetit]

@expez sure, I'm being lazy today, will do it the right way, sorry

[cichli]

Hmm, wrap-pprint doesn't actually handle a pprint op, but I can't think of any other way of achieving this. We could add a eval-pprint op, or maybe see if the describe op could be improved upstream to also describe this kind of middleware?

[cichli]

What does the describe op return if you add "eval" to handles instead of "pprint"?

[laurentpetit]

Indeed .... In the meantime I can test for another op as a workaround.

Le mardi 28 avril 2015, Michael Griffiths notifications@github.com a
écrit :

Hmm, wrap-pprint doesn't actually handle a pprint op, but I can't think
of any other way of achieving this. We could add a eval-pprint op, or
maybe see if the describe op could be improved upstream to also describe
this kind of middleware?

—
Reply to this email directly or view it on GitHub
#198 (comment)
.

Laurent Petit

[laurentpetit]

@cichli Haven't tested yet the highjacking of the "eval" op. Wouldn't that be entering a dangerous zone of implementation-specific behavior?

[laurentpetit]

We could name the op to make it clear that it describes a transverse feature and not a new op (ok even while writing it I'm not really convinced, bleh). What if we name the op "pprint-middleware" ?

[cichli]

What does the describe op return if you add "eval" to handles instead of "pprint"?

Ugh, don't know what I was thinking here. Obviously describe just returns the set of available ops, without the name of the descriptor that handles said op.

Yeah, I guess a dummy op makes sense here. Otherwise, this functionality was added in 0.9.0, so you could check the value of cider.nrepl.version/version - we do something similar after connecting in the emacs client, to ensure that the client and middleware versions are compatible. See here.

[laurentpetit]

I didn't know about the version check, that's great, thanks. Of course
theoretically cider could be present and the pprint middleware not
installed, but it will probably just work in 99.99% times.

Le mercredi 29 avril 2015, Michael Griffiths notifications@github.com a
écrit :

What does the describe op return if you add "eval" to handles instead of
"pprint"?

Ugh, don't know what I was thinking here. Obviously describe just returns
the set of available ops, without the name of the descriptor that handles
said op.

Yeah, I guess a dummy op makes sense here. Otherwise, this functionality
was added in 0.9.0, so you could check the value of
cider.nrepl.version/version - we do something similar after connecting in
the emacs client, to ensure that the client and middleware versions are
compatible. See here
https://github.com/clojure-emacs/cider/blob/c85d5ae23c134330a15a9638712c36835f448494/cider-interaction.el#L297-L304
.

—
Reply to this email directly or view it on GitHub
#198 (comment)
.

Laurent Petit

[laurentpetit]

If you're ok with a dummy op I can prepare a new version of the PR, along those lines:

• op named "pprint-middleware" or "dummy-pprint" or ?
• enhanced description explaining that it exposes the middleware presence but the op should not be called directly

Or, if in the future the middleware might also be called like "eval-pprint", then I could already name the op "eval-pprint" and in the description mention that it is not implemented yet, but that would be counter-intuitive I think.

[laurentpetit]

The last version does what Chas suggested: fixes the location for the :requires ... keys, and just creates a dummy middleware named after the middleware's var name: "pprint-middleware". All tests pass.

[bbatsov]

👍 Looks good to me. I hope you also opened that JIRA ticket about middleware discovery in nREPL. :-)

[laurentpetit]

JIRA ticket open on nrepl: http://dev.clojure.org/jira/browse/NREPL-77

[laurentpetit]

works like a charm with CCW, thanks!