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

Added setting to control the path of elm-format #162

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adiron
Copy link

@adiron adiron commented Aug 31, 2018

Some distributions of elm-format (e.g. Homebrew on Mac) give the
executable(s) names such as elm-format-0.19. Previously, the plugin
would try to run elm-format or else fail. Now, I have added
g:elm_format_command which can be set to any command/path/etc. so
elm-format can run even if installed under a weird path.

Some distributions of `elm-format` (e.g. Homebrew on Mac) give the
executable(s) names such as `elm-format-0.19`. Previously, the plugin
would try to run `elm-format` or else fail. Now, I have added
`g:elm_format_command` which can be set to any command/path/etc. so
`elm-format` can run even if installed under a weird path.
@jesse-c
Copy link
Contributor

jesse-c commented Oct 17, 2018

I wonder if this and #162 could be merged together?

We could also add the option to allow extra args, such as --elm-version like ALE allows.

@adiron
Copy link
Author

adiron commented Oct 18, 2018

Hey. Not sure which PR you're referring to, since this is #162 :P But either way I can try my best, though my viml is very basic

@jesse-c
Copy link
Contributor

jesse-c commented Oct 18, 2018

Haha, sorry, #146.

@adiron
Copy link
Author

adiron commented Oct 18, 2018

From the looks of it, that PR deals with basically the same thing this one deals with, so I'm fine with closing mine

@jesse-c
Copy link
Contributor

jesse-c commented Oct 18, 2018

I haven't had a proper look through the code, so I'm not sure which is a better implementation (though both are quite similar). @otaconix what do you think?

@otaconix
Copy link

otaconix commented Oct 18, 2018

The fact our two diffs are so alike gives me confidence that the approach I took is probably the right one. I'd be absolutely fine with this PR being chosen over mine.

Literally the only thing of note among the differences there are, is the name of the variable: elm_format_binary in my PR, elm_format_command in this one. Don't think you could go wrong with either.

@jesse-c
Copy link
Contributor

jesse-c commented Oct 19, 2018

Honestly it's up to you guys, maybe just flip a coin. 😂 #146 is older at least and has just slightly better comments.

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