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

[elixir] Add missing Alchemist commands and key bindings #8746

Closed
wants to merge 1 commit into from
Closed

[elixir] Add missing Alchemist commands and key bindings #8746

wants to merge 1 commit into from

Conversation

swaroopch
Copy link
Contributor

Commands and key bindings found via alchemist-refcard command.

I've tried to adhere to Spacemacs conventions but where I couldn't find relevant
conventions, I followed Alchemist's key bindings.

"tf" 'alchemist-test-file
"tn" 'alchemist-test-jump-to-next-test
"tp" 'alchemist-test-jump-to-previous-test
"tT" 'alchemist-project-run-tests-for-current-file
Copy link
Owner

Choose a reason for hiding this comment

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

What is the difference with alchemist-mix-test-file ?

Copy link
Contributor Author

@swaroopch swaroopch Apr 20, 2017

Choose a reason for hiding this comment

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

alchemist-mix-test-file will ask to choose which test file to run.

alchemist-project-run-tests-for-current-file will run tests for this file - if it cannot find the tests, it will ask whether to create it (y/n) and then create the corresponding file and open it in a new window for writing.

"oi" 'alchemist-macroexpand-once-region
"oI" 'alchemist-macroexpand-once-print-region
"or" 'alchemist-macroexpand-region
"oR" 'alchemist-macroexpand-print-region)
Copy link
Owner

@syl20bnr syl20bnr Apr 20, 2017

Choose a reason for hiding this comment

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

I don't know how macroexpand works for elixir but do you think this could be similar to macrostep for elisp ?
https://github.com/syl20bnr/spacemacs/blob/develop/layers/+lang/emacs-lisp/packages.el#L164-L175

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that the elixir macroexpand is not interactive, it is meant to be "expand once" or "expand completely" for different region sizes and with optional "printing" (appending below the current region as comments).

e.g. :
screen shot 2017-04-20 at 5 05 41 am

@swaroopch
Copy link
Contributor Author

Also, do we really need the maintenance of alchemist-refcard.tex? I'm not sure why that is needed since we already have a README, it would be great to not have to update documentation twice :)

@@ -91,10 +93,13 @@
"ta" 'alchemist-mix-test
"tb" 'alchemist-mix-test-this-buffer
"tt" 'alchemist-mix-test-at-point
"tf" 'alchemist-mix-test-file
"tf" 'alchemist-project-run-tests-for-current-file
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please note this breaking change... my goal was to make the key bindings more intuitive.

@syl20bnr
Copy link
Owner

Also, do we really need the maintenance of alchemist-refcard.tex? I'm not sure why that is needed since we already have a README, it would be great to not have to update documentation twice :)

It was a nice addition but you are right, we can delete them as they don't fit with the standard of Spacemacs so it feels weird to have this for only language.
PR welcome to remove them.

@swaroopch
Copy link
Contributor Author

@syl20bnr I have updated the PR to remove the alchemist-refcard.{tex,pdf} files.

I have been using this patch successfully for a few weeks now and have no issues so far.

Please let me know if any other changes are needed.

Thanks!

Copy link
Collaborator

@TheBB TheBB left a comment

Choose a reason for hiding this comment

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

Could use some squashing, but otherwise I'm ok with this.

@@ -201,10 +206,39 @@ You find and overview of all the key-bindings on the [[https://github.com/tonini
| ~SPC m x :~ | Run a custom execute command with =elixir= |
| ~SPC m x b~ | Run the current buffer through =elixir= |
| ~SPC m x f~ | Run =elixir= with the given filename |

Copy link
Collaborator

@JAremko JAremko May 19, 2017

Choose a reason for hiding this comment

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

One new line after a table is required :shipit:

https://travis-ci.org/syl20bnr/spacemacs/jobs/232505130#L478

@swaroopch
Copy link
Contributor Author

@TheBB I have squashed the branch.

@JAremko I have fixed the newline but now the build seems to complain about the ToC... I believe the ToC change was done automatically, not sure what I should be doing here, I'm on the latest develop branch, can you please guide me here?

@JAremko
Copy link
Collaborator

JAremko commented May 19, 2017

@swaroopch toc-org was updated recently snosov1/toc-org@421956e but CI uses the old version. @TheBB I think we need to merge this #8931 while I'm working on redesigning spacefmt

@JAremko JAremko mentioned this pull request May 19, 2017
@JAremko
Copy link
Collaborator

JAremko commented May 24, 2017

@swaroopch this PR has conflict 😢

Commands and key bindings found via `alchemist-refcard` command.

I've tried to adhere to Spacemacs conventions but where I couldn't find relevant
conventions, I followed Alchemist's key bindings.

Also, delete alchemist-refcard - approved by @syl20bnr at
#8746 (comment)
@swaroopch
Copy link
Contributor Author

@JAremko Rebased.

@TheBB
Copy link
Collaborator

TheBB commented May 26, 2017

Thanks! Cherry-picked in develop. You can safely delete your branch.

@TheBB TheBB closed this May 26, 2017
TheBB pushed a commit that referenced this pull request May 26, 2017
Commands and key bindings found via `alchemist-refcard` command.

I've tried to adhere to Spacemacs conventions but where I couldn't find relevant
conventions, I followed Alchemist's key bindings.

Also, delete alchemist-refcard - approved by @syl20bnr at
#8746 (comment)
@swaroopch swaroopch deleted the elixir-keys branch May 26, 2017 15:31
CodingSolo pushed a commit to CodingSolo/spacemacs that referenced this pull request Oct 29, 2018
Commands and key bindings found via `alchemist-refcard` command.

I've tried to adhere to Spacemacs conventions but where I couldn't find relevant
conventions, I followed Alchemist's key bindings.

Also, delete alchemist-refcard - approved by @syl20bnr at
syl20bnr#8746 (comment)
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.

None yet

4 participants