Add elixir-format function to format Elixir 1.6 files.

[anildigital]

elixir-format function formats Elixir 1.6 files with new mix format command which will be available from Elixir 1.6.

[anildigital]

@tonini Here is PR for elixir-format support in emacs-elixir. Let me know if you want any changes, I can do that. Hopefully this becomes part of emacs-elixir and see further contributions

[Trevoke]

Hi @anildigital - I'll get to this Monday, I promise.

[tonini]

Hi,

This looks nice, I would consider to add two add to things:

1. Add a test suite for it.
2. Include it into the emacs-* namespace something like elixir-format.....

[anildigital]

@tonini I don't have any clue to write a test suite. I will see if anyone can help me. @ckruse

[ckruse]

I‘m currently at my mother-in-law‘s house for a visit, I can have a look at Monday Via Mobile, deshalb kurz. Nicht unhöflich gemeint!

…

Am 18.02.2018 um 12:42 schrieb Anil Wadghule ***@***.***>: @tonini I don't have any clue to write a test suite. I will see if anyone can help me. @ckruse — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[anildigital]

@ckruse @jekku @Trevoke You can do changes in https://github.com/anildigital/mix-format.el so that I can send updated PR here. I will be adding you all as Co-Authors to this PR. Thanks.

[anildigital]

@Trevoke any update?

[Trevoke]

Hi @anildigital, I realize I didn't communicate well with you, I'm sorry. I'm a new maintainer of elixir-mode, and since Tonini beat me to the punch of requesting these changes, I thought you didn't need my input for now.

As far as the tests go, there are instructions in the CONTRIBUTING.md file and existing examples of tests you can use to guide yourself:

• https://github.com/elixir-editors/emacs-elixir/blob/master/CONTRIBUTING.md#how-to-run-tests
• https://github.com/elixir-editors/emacs-elixir/tree/master/test

[anildigital]

Thanks. I will follow these links. cc @ckruse

[draveness]

Hi, is this pull request ready to be merged?

[ckruse]

I'm not sure. I implemented tests and renamed everything to have a elixir-format- prefix. On my side everything seems to be ready for the merge.

[anildigital]

Hi @ckruse we need to merge changes done in https://github.com/anildigital/mix-format.el to this PR. But before that.. could you please send a PR to https://github.com/anildigital/emacs-elixir with tests. I this test-helper.el is conflicting there. Thanks.

[lucca65]

Hey! First of all, thanks for the effort on developing this feature! Any idea when this will be merged? Thanks!

[johnhamelink]

How would I test this in Spacemacs? 😄

[Trevoke]

That is a good question. My time has been taken in a heavy refactor of the alchemist.el backend - a part of that refactor involves enabling this feature. I could use a bunch of help. Once that's done, all you'll have to do in spacemacs or anywhere else would be updating alchemist.

On Mon, May 21, 2018, 14:11 John Hamelink ***@***.***> wrote: How would I test this in Spacemacs? 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#406 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEJSXUUMvMhZnkKnWRlp_NpH11fGtQEks5t0wNcgaJpZM4P5a1t> .

-- Phoned this one in.

[anildigital]

Hi @Trevoke @tonini, I have updated the PR with tests. The build is failing for an obvious reason, as tests are run against Elixir 1.0.5 version. Could you please check what can be done so that this PR can be merged. Thanks.

[Trevoke]

This should be paths.

[Trevoke]

Great, thank you for everyone's hard work.

I will figure out why the build is failing. In the meanwhile, @anildigital, I have a few requests:

1. Please make sure the elixir-format functions are available to anyone who requires elixir-mode directly - it does not make sense to provide the formatting functions as part of this major mode without making them directly available. Also please adjust the README accordingly.
2. Please change the README for the "format before save" section - it should just be the following:

;; Create a buffer-local hook to run elixir-format on save, only when we enable elixir-mode.
(add-hook 'elixir-mode-hook
          (lambda () (add-hook 'before-save-hook 'elixir-format nil t)))

3. Please delete the elixir-format-before-save function, as using it may cause emacs to behave in unexpected ways for users of this mode.
4. I'm unable to see in the tests how this code will behave if someone tries to call the format function with a version of Elixir that does not have the format task available. Which test covers this?

Thanks again for working on this!

[Trevoke]

@anildigital I have fixed the build. Please bring in the changes from this repo so you get the updated build config and Travis can do its work.

[anildigital]

@Trevoke I have made changes as requested. The build is still failing for some reason I could not figure out. I ran tests on macOS and Ubuntu OSes and tests ran fine.

Regarding #4 you mentioned, I think devs mostly will be coding with Elixir 1.6 now on. It would be overkill to check Elixir version everytime we call the elixir-format function. Also even if someone configures this plugin and tries to use Elixir 1.5 and earlier version, the mix format command would fail, giving an error. Hope this consideration is fine.

[Trevoke]

Hi @anildigital, thank you! Now we can see how the PR compares to the existing test suite.

Regarding calling elixir-format with an older version of Elixir, an error message displayed to the user is all I would like. What I was asking was, "where is the test that shows the error message to the user"? It sounds like that is already existing behavior, I just would like it formalized in a test so we can formalize what the expected behavior is from now on.

Re: the failures, you can find information about them by following this link:

https://travis-ci.org/elixir-editors/emacs-elixir/builds/384390061

When you ran your tests, what were the versions of emacs, elixir and erlang that you were using?
One thing to remember is that we must maintain multiple versions of emacs as well, so even if you had success with your own version of emacs, it is no guarantee that will work everywhere else.

Most of the build jobs are failures where some code receives nil and expects a string, and the build that's running with 1.6.0 has four tests failing:

FAILED elixir-format-should-message-on-error
FAILED elixir-format-should-run-hook-before-formatting
FAILED indents-a-buffer
FAILED indents-a-buffer-and-undoes-changes

This build is also running on emacs 24.3.

[anildigital]

Thanks a lot, @Trevoke, builds are passing now with all supported versions.

I spent some time to specially handle the case where elixir-format is run with Elixir version lesser than 1.6. It's a bit complicated to capture that as currently, we are messaging "elixir-format failed: ..." message when it fails. Could we add that feature as separate PR? Thanks.

[Trevoke]

Hi @anildigital, I was trying to work with the package and I didn't realize there was an orthogonal error. I fixed it on your side and now I'm completely OK with this being merged in as-is. It turned out I wasn't seeing the error buffer pop up and I was worried that it took away from usability. I really like the way this was put together!

Thank you and everyone else for all their hard work, I'm merging it in. I'll wait a little bit before creating a stable release on Melpa.