inf-clojure--make-single-line removes consecutive spaces in strings.

[austinhaas]

https://github.com/clojure-emacs/inf-clojure/blob/master/inf-clojure.el#L321

(count "1 5") => 3 ;; Should be 5.

EDIT: There are 3 spaces between 1 and 5 above. Apparently, this markdown is also removing the consecutive spaces.

[arichiardi]

Regexes are bad, I know, I did not cover that use. There are tests though, so a patch + tests should be easy to come up with (I guess?).

[hlolli]

The newlines are also removed from strings

=> "a   b"
"a b"
panaeolus.engine=> "a   
      
      b"
"a     b"
panaeolus.engine=> 

[sogaiu]

For inf-clojure-eval-region, this appears to have adverse affects when working with Arcadia (stalling of some sort).

Calling inf-clojure-eval-region seems to lead to the following chain of calls:

inf-clojure--send-string ->
inf-clojure--sanitize-command ->
inf-clojure--make-single-line

Is it necessary to modify a potentially multi-line region of text to be a single line for inf-clojure-eval-region?

I've been trying a modification where the original string from the region just has a newline appended to it and so far at least the stalling behavior seems better. I'm uncertain of whether the changes are appropriate.

[arichiardi]

Thanks for taking the time for looking into it. Yes newlines evaluate the command in comint so yes, some post processing must be done.

Having said that, it is far from perfect now so patches will be reviewed 😄 The reason is comint and that text-based REPLs are super tricky.

[sogaiu]

I confess to not understanding the explanation :) Are there specific bits of comint (or some document) I might look at to better understand this statement:

newlines evaluate the command in comint

?

I looked through comint.el for instances of newline, but didn't come away feeling any more informed...

[sogaiu]

@arichiardi, #40 seems related to this.

Therein, I see a quote from you:

that change would not work: that function has been added because we cannot send \ns or 
comint will evaluate the string. Each \n evaluates.

I presume that is what you were referring to by:

newlines evaluate the command in comint

There is also this statement:

The rationale of having a single line with no comment was because I could not send 
newlines without evaluating...

For regions, it doesn't appear impossible to send those that contain newlines and have them be evaluated, at least it seems to work with Arcadia. It seems to work when a region is selected from a non-repl buffer and that's sent (w/ a newline appended).

May be I'm not understanding the statement about "newlines evaluating" appropriately. Would you mind elaborating / spelling out some more of the details?

[arichiardi]

@sogaiu I think you understand it right.

Comint is character based and it evaluates as soon as it seems at \n if I remember correctly.

I have briefly checked comint.el and it seems that you can find a very nice explanation in comint-send-input.

Again, any improvement over this is greatly greatly appreciated as I don't have time myself for it at the moment.

[sogaiu]

Thanks for the hints.

Is it possible you mean that pressing the Enter / Return key triggers comint-send-input? Or do you mean something else?

[arichiardi]

It might be, I have very blurred memories about how that all works, your best bet is to probably try directly with ielm. Are you familiar with elisp a bit? If so it should be an easy one.

[sogaiu]

FWIW, I verified with Wireshark that pasting a region into shell mode (connecting to a socket REPL with nc) does not lead to evaluation. I waited some minutes before pressing Enter, which did lead to the entire region being sent by comint-send-input.

Thanks for the tip about ielm -- I am not familiar with it, but will investigate.

[arichiardi]

In that case probably then we wouldn't need to get rid of the \n in the whole string.

[sogaiu]

What would break if inf-clojure--send-string sends string + "\n" via comint-send-string instead of sanitized?

[sogaiu]

With that change, the following:

austinhaas' count - #152 (comment)
hlolli's newline example - #152 (comment)

appear to yield the correct results here.

inf-clojure-eval-region also appears to work so far.

On a side note, the long string example in the original posting of #40 also works properly here -- though this I doubt the environment (Arcadia / Clojure CLR) I'm testing in is the same as that of the original poster.

[arichiardi]

We could try and see how many people report breakage? I seem to remember lumo was breaking but maybe with a patch I can double check if it still true. Definitely a patch to try out would be great.

[sogaiu]

I tried with lumo:

Lumo 1.8.0
ClojureScript 1.9.946
Node.js v9.2.0

using its socket repl, and it seemed ok in limited testing. Anyway, below is a patch:

diff --git a/inf-clojure.el b/inf-clojure.el
index 84eac7d..c6a5b02 100644
--- a/inf-clojure.el
+++ b/inf-clojure.el
@@ -355,7 +355,7 @@ customizations."
   (inf-clojure--set-repl-type proc)
   (let ((sanitized (inf-clojure--sanitize-command string)))
     (inf-clojure--log-string sanitized "----CMD->")
-    (comint-send-string proc sanitized)))
+    (comint-send-string proc (concat string "\n"))))
 
 (defcustom inf-clojure-load-form "(clojure.core/load-file \"%s\")"
   "Format-string for building a Clojure expression to load a file.

[arichiardi]

Ah yes now I seem to remember better now, there might be a problem with the character-based setup. Socket REPL has always worked like a charm. Anyways I will try it out thank you!

[sogaiu]

Ah, I don't think I've used anything other than the socket mode code.

FWIW, I just tried (setq inf-clojure-generic-cmd "lumo -d") followed by M-x inf-clojure. Would that count as a character-based setup? In any case I tried evaluating a few regions with multiple forms under those conditions without noticing any problems.

Good luck! I hope things work out :)

[arichiardi]

Yes that's exactly the setup I meant to try so thanks for trying!

It is obviously solving a lot of things it seems.

[candera]

Bump. Ran into this today. Pain to track down.

[arichiardi]

Hi Craig in which REPL?

The patch seems to fix things in lumo but I am hesitant to bring it in unless we test all the others...I have also been pretty busy and didn't have to much time so it would be really awesome to try this patch against the other REPL types.

[candera]

I tried it with boot and with clojure, both via and not via a socket. I don't use lein.

I just want to reiterate how critically wrong the current behavior is, whether the fix is the patch mentioned above or not, to help contextualize effort for a fix. Altering the transmitted forms is completely broken. In its current form it evaluates "a b c" to "a b c". Consider that this has the effect of making (= "a b" "a b") true, among many other issues. It's sort of like if (+ 1 2) evaluated to 5.

[arichiardi]

@candera do you mean you tried the fix already or you are reporting the wrong behavior only?

I will try to take some for this myself but if someone tests the fix with a least boot and clojure I guess we can call it good.

[candera]

Yes, sorry for the ambiguity .I tried the fix above (#152 (comment)) with both boot and clojure.

[arichiardi]

Cool thank you

[austinhaas]

It's been a while since I looked at this, but IIRC, there were a couple places where inf-clojure altered the input and the output to/from the REPL. I didn't want inf-clojure to touch any of that. I got the impression that the code was based on assumptions about the format for one particular REPL. I am not interested in REPLs other than the ones that come with Clojure/Clojurescript. So, I ended up forking inf-clojure and changing the input/output handling for my needs. I didn't want to have to update and test the other REPLs I don't use. That version is here: https://github.com/austinhaas/inf-clojure/tree/io

It has been working well for me. The main defect is that I made it write all output to the minibuffer and that can get noisy if you are evaluating something with a lot of output. I'm not sure what the solution is, but I'd rather have the noise than no output in the minibuffer.

Another example of a philosophical difference, I think: if I evaluate a whole buffer, or a region, then I want that whole block of text, including all of the newlines, to be sent to the REPL. That means the output, with the stock Clojure REPL anyway, will have a bunch of empty line responses. I'm fine with that. I just want inf-clojure to send exactly what I selected.

[arichiardi]

@austinhaas and folks

Just to be clearz 😄

I had a problem and I found that a solution to that problem was that hack. How many times in your programming career have you done that? You really really try not to but I found that it was working for all the REPLs and we merged it in.

I see this thread and unfortunately the only thing I see is an inline patch, a fork and complaints.

@austinhaas is your post supposed to mean you would like to maintain the alternative fork of inf-clojure? If yes, that's really awesome and I would contribute patches when I have the time..

If not, this is kind of getting nowhere and unless someone steps up and does the work and fixes and tests for all the REPLs I guess there no need to add anything more. Don't we think?

😉

[austinhaas]

@arichiardi, no, I'm not volunteering to become the maintainer of an alternate fork. I was just adding information in case it was helpful to anyone. I'm sorry if that came off as imprudent.

[dotemacs]

To give a helpful nudge, I've applied @sogaiu's patch and tried out the following at the REPL:

(count "1   5")

NOTE: there is three (3) spaces between 1 and 5 in the above code snippet. Five characters in total.

The code above returns 3 prior to the patch and 5 after the patch, at the REPL of:

• ClojureCLR 1.9.0
• Clojure 1.10.0
• Clojure 1.9.0
• Clojure 1.8.0
• Clojure 1.7.0
• Planck 2.10.0
• Lumo 1.7.0
• Joker 0.12.3

If the above are sufficient test case, @sogaiu would you be able to submit a pull request?

[arichiardi]

I actually have a patch for this, I just need now to finish it up. This weekend is tough, probably next week.

And thanks for checking of course 😀

[arichiardi]

Opened a PR for this, didn't want to go all the way but I got rid of most of the trouble I think.