Skip to content

Conversation

Malabarba
Copy link
Member

Fix #1011.

Something like this?
Not sure where to bind it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's be better to move this inlined code to a simple middleware op.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which middleware would you put it in? info? ns?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But Idk... Isn't this a little narrow for a middleware?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd put it in ns. Middleware shouldn't be complex - it should just get some work done. The problem with evaluating strings is twofold:

  1. The meaning of the code is obscured by implementation details
  2. Even simple code can be problematic (in your case you're not using FQ names, which means that in some namespaces this code will misbehave)

@Malabarba
Copy link
Member Author

All added. Along with clojure-emacs/cider-nrepl#221

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, shouldn't this cover the option of function being used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. :-)

@Malabarba
Copy link
Member Author

Tests added and name fixed.

bbatsov added a commit that referenced this pull request Jun 30, 2015
Define command to run -main
@bbatsov bbatsov merged commit 1709246 into clojure-emacs:master Jun 30, 2015
@bbatsov
Copy link
Member

bbatsov commented Jun 30, 2015

👍

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

Successfully merging this pull request may close these issues.

2 participants