Split cider-interaction into smaller libraries #1068

Closed
bbatsov opened this Issue Apr 4, 2015 · 8 comments

Comments

Projects
None yet
3 participants
@bbatsov
Member

bbatsov commented Apr 4, 2015

Right now cider-interaction.el is the dumping ground for all sorts of unrelated functionality. It was always my intention for it to exist only temporarily and to be superseded by small more focused libraries.

Not sure when/if I'll get to doing this myself. In the mean time - help is welcome!

@vspinu

This comment has been minimized.

Show comment
Hide comment
@vspinu

vspinu Apr 7, 2015

Contributor

A preliminary proposal here:

  • Move all doc functions into cider-doc.el, possibly with some refactoring
  • Move all completion functionality into cider-completion.el (merge with cider-eldoc.el?)
  • Move low level nrepl interaction functionality into cider-client.el
  • Move repl related stuff into cider-repl.el
  • Leave cider-eval-* and cider-find-* stuff in cider-interaction.el

IMO eldoc is much more related to completion than to documentation and having it together with other completion would make it easier to reuse caching engine if any.

Contributor

vspinu commented Apr 7, 2015

A preliminary proposal here:

  • Move all doc functions into cider-doc.el, possibly with some refactoring
  • Move all completion functionality into cider-completion.el (merge with cider-eldoc.el?)
  • Move low level nrepl interaction functionality into cider-client.el
  • Move repl related stuff into cider-repl.el
  • Leave cider-eval-* and cider-find-* stuff in cider-interaction.el

IMO eldoc is much more related to completion than to documentation and having it together with other completion would make it easier to reuse caching engine if any.

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Apr 7, 2015

Member

Move all doc functions into cider-doc.el, possibly with some refactoring

Most of them are already there.

I'd actually prefer a refactoring that would split the library into more autoloadable parts (cider-apropos and cider-eldoc are a good example).

Move all completion functionality into cider-completion.el

Fine by me.

IMO eldoc is much more related to completion than to documentation and having it together with other completion would make it easier to reuse caching engine if any.

There's no longer need for this. Let's keep eldoc as is.

Move repl related stuff into cider-repl.el

This is more or less the case even now.

Leave cider-eval-* and cider-find-* stuff in cider-interaction.el

I'd prefer killing cider-interaction altogether - it can be replaced by something like cider-core and cider-misc. The name is an unfortunately accident - we used to have a mode called nrepl-interaction-mode and when I split the original nrepl.el I called one of the files nrepl-interaction. It was supposed to be a temp setup, but here we are 2 years later. :-)

Member

bbatsov commented Apr 7, 2015

Move all doc functions into cider-doc.el, possibly with some refactoring

Most of them are already there.

I'd actually prefer a refactoring that would split the library into more autoloadable parts (cider-apropos and cider-eldoc are a good example).

Move all completion functionality into cider-completion.el

Fine by me.

IMO eldoc is much more related to completion than to documentation and having it together with other completion would make it easier to reuse caching engine if any.

There's no longer need for this. Let's keep eldoc as is.

Move repl related stuff into cider-repl.el

This is more or less the case even now.

Leave cider-eval-* and cider-find-* stuff in cider-interaction.el

I'd prefer killing cider-interaction altogether - it can be replaced by something like cider-core and cider-misc. The name is an unfortunately accident - we used to have a mode called nrepl-interaction-mode and when I split the original nrepl.el I called one of the files nrepl-interaction. It was supposed to be a temp setup, but here we are 2 years later. :-)

@cichli

This comment has been minimized.

Show comment
Hide comment
@cichli

cichli Apr 7, 2015

Member

Yep, all makes sense. Including cider-complete-at-point in cider-complete.el will mean that file depends on cider-eldoc.el (needed for cider-company-docsig) - we can just have a tiny cider-capf.el containing the relevant bits for that if we really want to keep them independent.

I'm also not sure if the bits for formatting and tracing are deserving of their own files or they should just go in misc.

I'll start tackling this issue (along with the compilation warnings, see #875) this week.

Member

cichli commented Apr 7, 2015

Yep, all makes sense. Including cider-complete-at-point in cider-complete.el will mean that file depends on cider-eldoc.el (needed for cider-company-docsig) - we can just have a tiny cider-capf.el containing the relevant bits for that if we really want to keep them independent.

I'm also not sure if the bits for formatting and tracing are deserving of their own files or they should just go in misc.

I'll start tackling this issue (along with the compilation warnings, see #875) this week.

@vspinu

This comment has been minimized.

Show comment
Hide comment
@vspinu

vspinu Apr 7, 2015

Contributor

it can be replaced by something like cider-core and cider-misc.

These are among the most non-suggestive names I could think of :) At least interaction stands for interacting with the process, and things like "send code to" (aka eval), "set-ns", "fiind var" etc can be easily called interactions.

Over-shattering into small files almost always leads to a nightmare with circular dependencies. Which in turn leads to more "util" files like cider-capf.el proposed by @cichli.

Contributor

vspinu commented Apr 7, 2015

it can be replaced by something like cider-core and cider-misc.

These are among the most non-suggestive names I could think of :) At least interaction stands for interacting with the process, and things like "send code to" (aka eval), "set-ns", "fiind var" etc can be easily called interactions.

Over-shattering into small files almost always leads to a nightmare with circular dependencies. Which in turn leads to more "util" files like cider-capf.el proposed by @cichli.

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Apr 7, 2015

Member

These are among the most non-suggestive names I could think of :) At least interaction stands for interacting with the process, and things like "send code to" (aka eval), "set-ns", "fiind var" etc can be easily called interactions.

The problem is you can call pretty much everything an interaction. Let's start the splitting and we'll have a better idea what makes sense and what doesn't.

I'm also not sure if the bits for formatting and tracing are deserving of their own files or they should just go in misc.

Formatting probably doesn't need a separate library, but tracing can be extracted I guess (I planned to do so when I added different commands for namespaces and vars).

Member

bbatsov commented Apr 7, 2015

These are among the most non-suggestive names I could think of :) At least interaction stands for interacting with the process, and things like "send code to" (aka eval), "set-ns", "fiind var" etc can be easily called interactions.

The problem is you can call pretty much everything an interaction. Let's start the splitting and we'll have a better idea what makes sense and what doesn't.

I'm also not sure if the bits for formatting and tracing are deserving of their own files or they should just go in misc.

Formatting probably doesn't need a separate library, but tracing can be extracted I guess (I planned to do so when I added different commands for namespaces and vars).

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Apr 7, 2015

Member

One more comment on the topic - we might actually merge cider.el with cider-interaction (or whatever is left from it), as right now cider.el is pretty much an empty shell.

Member

bbatsov commented Apr 7, 2015

One more comment on the topic - we might actually merge cider.el with cider-interaction (or whatever is left from it), as right now cider.el is pretty much an empty shell.

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Sep 12, 2015

Member

@cichli You mentioned a while back you planned to work on this. Any progress? With all the ongoing refactoring it'd be nice to fit this in 0.10 as well.

Member

bbatsov commented Sep 12, 2015

@cichli You mentioned a while back you planned to work on this. Any progress? With all the ongoing refactoring it'd be nice to fit this in 0.10 as well.

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Jul 10, 2016

Member

I'm guessing we can close this, as nothing focused has been done on this front in quite a while.

Member

bbatsov commented Jul 10, 2016

I'm guessing we can close this, as nothing focused has been done on this front in quite a while.

@bbatsov bbatsov closed this Jul 10, 2016

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