Rework the REPL to use comint #709

Closed
shosti opened this Issue Aug 7, 2014 · 20 comments

Comments

Projects
None yet
8 participants
@shosti

shosti commented Aug 7, 2014

This is a really small thing, but the current behavior for the CIDER repl is to always scroll to show maximum output. I don't really like this behavior, and it would be nice if it were configurable (I'm currently hacking around it by redefining cider-repl--show-maximum-output—let me know if there's already another way to turn this behavior off). A cider-repl-show-maximum-output variable would probably be enough, although it might be nice to have some more configuration options à la comint—what are your thoughts? (Also, are there any plans to eventually base cider-repl-mode off of comint-mode?) I'd be happy to put together a PR for this.

@vspinu

This comment has been minimized.

Show comment
Hide comment
@vspinu

vspinu Aug 7, 2014

Contributor

👍 I would love if cider repl could behave similarly to comint buffers and would use comint customization variables.

Why exactly cider decided not to base its repl on comint?

Contributor

vspinu commented Aug 7, 2014

👍 I would love if cider repl could behave similarly to comint buffers and would use comint customization variables.

Why exactly cider decided not to base its repl on comint?

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Aug 7, 2014

Member

The REPL code was originally copied from SLIME. It'd be a huge improvement if we switched to comint, but I don't have time to work on that. Help welcome!

Member

bbatsov commented Aug 7, 2014

The REPL code was originally copied from SLIME. It'd be a huge improvement if we switched to comint, but I don't have time to work on that. Help welcome!

@shosti

This comment has been minimized.

Show comment
Hide comment
@shosti

shosti Aug 7, 2014

I'd be happy to take a look at moving the repl code to comint, although it might take a little while (I'm a little busy right now and I'm not an expert in comint).

shosti commented Aug 7, 2014

I'd be happy to take a look at moving the repl code to comint, although it might take a little while (I'm a little busy right now and I'm not an expert in comint).

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Aug 7, 2014

Member

geiser's and ielm's source code can used as reference I guess, as they both currently employ comint.
Using comint would also solve issues like #326.

Member

bbatsov commented Aug 7, 2014

geiser's and ielm's source code can used as reference I guess, as they both currently employ comint.
Using comint would also solve issues like #326.

@bbatsov bbatsov changed the title from Make repl auto-scrolling configurable to Rework the REPL to use comint Aug 7, 2014

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Sep 15, 2014

Member

@Vitoshka I think you can find some useful ideas about the comint implemenation here. Sly is a new fork of SLIME using comint based REPL buffers.

Member

bbatsov commented Sep 15, 2014

@Vitoshka I think you can find some useful ideas about the comint implemenation here. Sly is a new fork of SLIME using comint based REPL buffers.

@vspinu

This comment has been minimized.

Show comment
Hide comment
@vspinu

vspinu Sep 15, 2014

Contributor

Thanks for the pointer. I will have a look at it when I move to comint next week. There are still a couple things I would like to push to CIDER before getting to comint.

Contributor

vspinu commented Sep 15, 2014

Thanks for the pointer. I will have a look at it when I move to comint next week. There are still a couple things I would like to push to CIDER before getting to comint.

@sooheon

This comment has been minimized.

Show comment
Hide comment
@sooheon

sooheon Dec 11, 2014

Another quality of life improvement if this were to happen would be interaction with evil-mode. Currently, actions such as escaping out of insert mode into normal mode place the cursor at the start of the REPL prompt. This is a problem when you then send snippets of code from the clojure buffer, it gets inserted before the prompt. Commands like evil-delete-line do not work, while comint based packages like monroe or inf-clojure handle this just fine.

sooheon commented Dec 11, 2014

Another quality of life improvement if this were to happen would be interaction with evil-mode. Currently, actions such as escaping out of insert mode into normal mode place the cursor at the start of the REPL prompt. This is a problem when you then send snippets of code from the clojure buffer, it gets inserted before the prompt. Commands like evil-delete-line do not work, while comint based packages like monroe or inf-clojure handle this just fine.

@env0der

This comment has been minimized.

Show comment
Hide comment
@env0der

env0der Dec 13, 2014

Yes, integration with evil-mode in repl would be awesome.

env0der commented Dec 13, 2014

Yes, integration with evil-mode in repl would be awesome.

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Dec 13, 2014

Member

Rebasing cider-repl-mode on top of comint-mode should solve your problems. I don't think we'll need any more evil-specific support.

Member

bbatsov commented Dec 13, 2014

Rebasing cider-repl-mode on top of comint-mode should solve your problems. I don't think we'll need any more evil-specific support.

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Aug 11, 2015

Member

This important ticket is still looking for a volunteer to tackle it.

Member

bbatsov commented Aug 11, 2015

This important ticket is still looking for a volunteer to tackle it.

@GordonGustafson

This comment has been minimized.

Show comment
Hide comment
@GordonGustafson

GordonGustafson Aug 13, 2015

Contributor

@vspinu did you have any unfinished work on this I should look at if I decide to take a crack at this? Or should I start by looking at ielm source and the comint documentation?

Contributor

GordonGustafson commented Aug 13, 2015

@vspinu did you have any unfinished work on this I should look at if I decide to take a crack at this? Or should I start by looking at ielm source and the comint documentation?

@johnbendi

This comment has been minimized.

Show comment
Hide comment
@johnbendi

johnbendi Aug 13, 2015

What about @shosti the OP who promised a PR?

What about @shosti the OP who promised a PR?

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Aug 13, 2015

Member

I think that nobody actually got anywhere with this task, so @GordonGustafson can definitely start from scratch.

Member

bbatsov commented Aug 13, 2015

I think that nobody actually got anywhere with this task, so @GordonGustafson can definitely start from scratch.

@vspinu

This comment has been minimized.

Show comment
Hide comment
@vspinu

vspinu Aug 13, 2015

Contributor

@GordonGustafson, I haven't done anything concrete.

Contributor

vspinu commented Aug 13, 2015

@GordonGustafson, I haven't done anything concrete.

@shosti

This comment has been minimized.

Show comment
Hide comment
@shosti

shosti Aug 13, 2015

@GordonGustafson yeah, never got around to looking at this (as you can probably surmise).

shosti commented Aug 13, 2015

@GordonGustafson yeah, never got around to looking at this (as you can probably surmise).

@bmag bmag referenced this issue in syl20bnr/spacemacs Dec 7, 2015

Closed

Saving history for sub-shells #4039

@johnv02139

This comment has been minimized.

Show comment
Hide comment
@johnv02139

johnv02139 Aug 25, 2016

Contributor

I think basing it off of comint is a great idea, but for the record, for the main complaint, I added a defcustom in January, cider-repl-scroll-on-output, which can be set to nil to avoid the irritating scrolling. I didn't see this issue at the time.

Is any work ongoing here to use comint?

Contributor

johnv02139 commented Aug 25, 2016

I think basing it off of comint is a great idea, but for the record, for the main complaint, I added a defcustom in January, cider-repl-scroll-on-output, which can be set to nil to avoid the irritating scrolling. I didn't see this issue at the time.

Is any work ongoing here to use comint?

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Aug 26, 2016

Member

To my knowledge no one is working on this, so without a volunteer to push it forward it's not going to happen any time soon.

Member

bbatsov commented Aug 26, 2016

To my knowledge no one is working on this, so without a volunteer to push it forward it's not going to happen any time soon.

@vspinu

This comment has been minimized.

Show comment
Hide comment
@vspinu

vspinu Aug 26, 2016

Contributor

I am not even convinced that it's worth anymore Comint is old and rigid which makes it hard to build a richer interfaces on top of it. So, IMO, if there is ever an effort in this direction, it should be better spent on building a new comint system from scratch.

Contributor

vspinu commented Aug 26, 2016

I am not even convinced that it's worth anymore Comint is old and rigid which makes it hard to build a richer interfaces on top of it. So, IMO, if there is ever an effort in this direction, it should be better spent on building a new comint system from scratch.

@johnv02139

This comment has been minimized.

Show comment
Hide comment
@johnv02139

johnv02139 Aug 26, 2016

Contributor

I would agree it doesn't seem worth reimplementing. I think your (@vspinu) original message made more sense, i.e., "use comint customization variables." I would think we could use the variables and emulate the behavior without sharing implementation.

Contributor

johnv02139 commented Aug 26, 2016

I would agree it doesn't seem worth reimplementing. I think your (@vspinu) original message made more sense, i.e., "use comint customization variables." I would think we could use the variables and emulate the behavior without sharing implementation.

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Oct 2, 2016

Member

OK, I'll close this ticket then. We've got more important tasks to do anyways.

Member

bbatsov commented Oct 2, 2016

OK, I'll close this ticket then. We've got more important tasks to do anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment