Skip to content
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

Clojure enhancements #11431

Closed
wants to merge 3 commits into from
Closed

Conversation

kommen
Copy link
Contributor

@kommen kommen commented Oct 12, 2018

  1. add a keybinding to open the cider repl history buffer in an evil way (see Clojure enhancements #11431 (comment))
  2. evilfy cider-repl-history-mode-map
  3. allow to send input to a cider repl in normal mode with RET

@@ -229,7 +229,8 @@

(evil-define-key 'normal cider-repl-mode-map
(kbd "C-j") 'cider-repl-next-input
(kbd "C-k") 'cider-repl-previous-input)
(kbd "C-k") 'cider-repl-previous-input
(kdb "RET") 'cider-repl-return)
Copy link
Contributor

@practicalli-johnny practicalli-johnny Oct 13, 2018

Choose a reason for hiding this comment

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

Yes, this keybinding will make a vast improvement to the usability of the Clojure repl when using Evil

Copy link
Collaborator

@d12frosted d12frosted left a comment

Choose a reason for hiding this comment

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

Nice! But please add new key bindings to the clojure layer readme file. There is a dedicated section for key bindings. 😸

@kommen
Copy link
Contributor Author

kommen commented Oct 13, 2018

@d12frosted I discussed with @jr0cket on the Clojurians Slack Spacemacs channel about putting cider-repl-history onto SPC s h instead of , P. I used , P since cider has it on C-c M-p, but I can see making since having it under SPC s since there all the repl commands are. Any opinions on that?

@practicalli-johnny
Copy link
Contributor

We definately need keybindings for Vim Normal mode as it feels very inconsistent changing into vim insert mode just to call the repl history with ,. Its also easy to get confused between , in insert and normal mode. Having to calle SPC SPC cider-repl-history is not great either.

I suggested , s h as it fits in with the mnemonic menu approach: major-mode > cider > history.

Although P is used in the cider package itself, several of the keybindings have changed to conform to the mnemonic approch. If there were other history related keybindings already using P then it would make sense, but I havent seen this convention in Spacemacs.

@kommen
Copy link
Contributor Author

kommen commented Oct 20, 2018

I've removed the , P key for now seems it seems this needs some more discussion. This still leaves the current way of getting to the history via the cider repl shortcuts, which I just discovered now myself:
in a repl buffer in vim insert mode: , then select history
in a repl buffer in vim normal mode: , , then select history

So I'm not sure if we even should add custom keybindings.

@jr0cket I've also addressed the issue you ran into that RET in vim normal mode didn't evaluate: I had I typo on the patch.

@d12frosted Since this PR doesn't introduce any new keybindings now, I think no changes to the README are needed. The RET keybinding just makes it behave as it should have been and RET is not documented for vim input mode in the repl as well.

@d12frosted
Copy link
Collaborator

@kommen makes sense. I've tried to search for any other layer that defines key bindings for navigating the REPL history and didn't find one. But since REPL usually has it's own mode, I don't see a reason to add key bindings under , s.

Anyway, I've cherry-picked this commit into develop, you can safely delete your branch. Thank you 🎉

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.

3 participants