Skip to content

All strings sent to REPL are converted from multi-line to single-line #187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
RobertARandolph opened this issue Feb 13, 2021 · 10 comments
Closed
Assignees

Comments

@RobertARandolph
Copy link
Contributor

Expected behavior

Text sent to REPL is sent as-is, without conversion or parsing.

I view this as expected behaviour given that this is traditionally how inferior-lisp works, and the standard clojure REPL supports multiline strings without issue.

Actual behavior

inf-clojure converts all strings to a single-line before sending to the comint process.

This causes the 1024 TTY character limit per line problem on macOS to be frequently encountered in multiple scenarios.

There are at least 3 related issues to this line of code:

#152
#40
#183

and a branch with a (strange) potential fix:

https://github.com/clojure-emacs/inf-clojure/tree/fix-152

Steps to reproduce the problem

Unnecessary.

Other

I've been using inf-clojure with the referenced code (and associated calls) removed for 2 weeks full-time without issue in clj and cljs repls. I used babashka for a short period of time without issue as well.

I have not tried planck/lumo/joker.

@bbatsov
Copy link
Member

bbatsov commented Feb 13, 2021

It took me a while, but I found the original problem - #7

@dpsutton dpsutton self-assigned this Feb 13, 2021
@arichiardi
Copy link
Contributor

arichiardi commented Feb 13, 2021

If memory serves me well lumo is the one that caused problems. The fix looks good to me.

Edit, lol sorry I haven't had time to finish that up

@bbatsov
Copy link
Member

bbatsov commented Feb 13, 2021

No worries. I had forgotten about that patch, but I see that it bring back the subprompt issue in certain cases. If it only impacts Lumo, I guess that's fine - from what I know it's pretty much dead these days.

@dpsutton
Copy link
Contributor

i believe it is officially deprecated. i suppose if we wanted to we could make this an option, automatically on for lumo, or just drop it entirely.

@arichiardi
Copy link
Contributor

Agree we should proceed with the patch and special case for lumo maybe, in order not to break people...my 2c

@RobertARandolph
Copy link
Contributor Author

Has it been tried with Lumo? I can't even get Lumo to run on my machine.

@arichiardi
Copy link
Contributor

I tried it when I wrote my last comment in the PR. An if on the repl type should be enough. The other option is breaking, which probably is ok as long as we write this in the Changelog (or maybe Bozhidar has a better idea)

@bbatsov
Copy link
Member

bbatsov commented Feb 13, 2021

Well, let's merge your patch and see what happens.

@RobertARandolph
Copy link
Contributor Author

Update after a week is that the merged fix has been working fine for me, and a few other people I know that I are using it actively.

@bbatsov
Copy link
Member

bbatsov commented Feb 19, 2021

Good to know! Thanks for following up!

@bbatsov bbatsov closed this as completed Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants