Executing cider-pprint-eval-last-sexp evaluates sexp twice

[marcinwaldowski]

Expected behavior

Executing cider-pprint-eval-last-sexp in cljc file when cider-request-dispatch is set to 'static with one (Clojure type) only repl connection should evaluate sexp once.

Actual behavior

Executing cider-pprint-eval-last-sexp in cljc file when cider-request-dispatch is set to 'static with one (Clojure type) only repl connection evaluates sexp twice.

Steps to reproduce the problem

Set cider-request-dispatch to 'static

Type in cljc file:

(do (print "Hello! ")
   (+ 1 1))

Execute: cider-pprint-eval-last-sexp

The popup buffer will contain:

Hello! 2
Hello! 2

Environment & Version information

CIDER 0.15.1 (London), nREPL 0.2.12
Leiningen 2.7.1
GNU Emacs 25.3.2 (x86_64-pc-linux-gnu, GTK+ Version 3.10.8) of 2017-09-12
Clojure 1.9.0-alpha17, Java 1.8.0_144
elementary OS Freya 0.3.2
4.4.0-96-generic #119~14.04.1-Ubuntu SMP Wed Sep 13 08:40:48 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

[vspinu]

Is this in cljc file by chance? This is because the code is executed in both repls. I think this is the correct behavior.

[marcinwaldowski]

Thanks for your answer. I found that it is really related to cljc file only. I investigated it more and I found that if I have only one repl connection (with type Clojure) and I execute cider-pprint-eval-last-sexp on cljc file then :

• if variable cider-request-dispatch is set to 'dynamic then sexp is evaluated only once,
• if variable cider-request-dispatch is set to 'static then sexp is is evaluated twice.

The behavior for cider-request-dispatch set to 'static looks incorrect for me.

[marcinwaldowski]

I changed issue description with information that it happens only for cljc files and if cider-request-dispatch is set to 'static.

[bbatsov]

Yeah, that's certainly a bug and I think it should pretty easy to fix.

[xiongtx]

Yeah, that's certainly a bug

According to @vspinu this is expected behavior? What should the behavior be?

[bbatsov]

if variable cider-request-dispatch is set to 'dynamic then sexp is evaluated only once,
if variable cider-request-dispatch is set to 'static then sexp is is evaluated twice.

Well, static dispatch means that code should be evaluated only in whatever is currently active connection, and you can't have two of those.

I'm actually surprised this is done 1 time for dynamic dispatch and 2 times for static - I would have expected it to be the other way around, as dynamic dispatch would normally figure out that 2 connections are an option for some evaluation in a cljc file (if both the clj and cljs) connection are already present that is.

Probably we should make it explicitly configurable what happens when you evaluate code in cljc files, though. There are too many assumptions about what people need right now.

[vspinu]

I'm actually surprised this is done 1 time for dynamic dispatch and 2 times for static

Indeed. It should be the other way around. I will look into this today along with the other connection related issue that I promised to fix.

[vspinu]

Checked it up. The behavior with the dynamic dispatch is as expected, the code is evaluated once for each dialect (once in clj buffer and once in cljs buffer, so twice in total). With static dispatch the code is evaluated twice in clj buffer. So static dispatch is broken then.

Fixing this would require adding yet another hack to the pile of hacks already in place, and I am not particularly keen on it. IMO static dispatch should be deprecated and connection system rewritten, but I guess you already know my take on this ;)

[xiongtx]

static dispatch should be deprecated and connection system rewritten, but I guess you already know my take on this ;)

So this problem should go away when #2069 is merged?

[vspinu]

Well, #2069 removes static dispatch, so yes it "fixes" it.

[bbatsov]

Fixing this would require adding yet another hack to the pile of hacks already in place, and I am not particularly keen on it. IMO static dispatch should be deprecated and connection system rewritten, but I guess you already know my take on this ;)

I guess we do. Well, it'd be nice if we still fix this in scope of 0.16, as the connection handling changes are certainly not going to happen before 0.17. I can't imagine fixing this is particularly complex.

[xiongtx]

as the connection handling changes are certainly not going to happen before 0.17

😭

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[stale]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.