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

Forcing globally installed elm-format #10

Closed
krksgbr opened this issue Apr 17, 2019 · 5 comments
Closed

Forcing globally installed elm-format #10

krksgbr opened this issue Apr 17, 2019 · 5 comments

Comments

@krksgbr
Copy link

krksgbr commented Apr 17, 2019

Hi!
In our project, elm-format is installed as a nix package. This means that in nix-shell we need to be able to run the instance of elm-format provided by nix instead of the one installed by prettier-plugin-elm as a dependency. We'd like to have a way of configuring this behavior. Not sure what the best way would be for passing in configuration, maybe through an environment variable: PRETTIER_PLUGIN_ELM_PREFER_LOCAL=false? Currently we solve this problem through a fork, but it would be nice to have this supported in the official package. Would you accept a PR for this?

@kachkaev
Copy link
Member

kachkaev commented Apr 17, 2019

Hi @krksgbr! I see what you're after.

How about an env variable called PRETTIER_PLUGIN_ELM_CUSTOM_PATH_TO_ELM_FORMAT? When it's set to something, we do this:

export const formatTextWithElmFormat = (text: string): string => {
return execa.sync("elm-format", ["--stdin", "--elm-version=0.19"], {
input: text,
preferLocal: true,
localDir: __dirname,
stripEof: false,
}).stdout;
};

export const formatTextWithElmFormat = (text: string): string => {
  const customPathToElmFormat = process.env.PRETTIER_PLUGIN_ELM_CUSTOM_PATH_TO_ELM_FORMAT;
  return execa.sync(customPathToElmFormat || "elm-format", ["--stdin", "--elm-version=0.19"], {
    input: text,
    preferLocal: !!customPathToElmFormat,
    localDir: __dirname,
    stripEof: false,
  }).stdout;
};

What you'll then be able to do is:

export PRETTIER_PLUGIN_ELM_CUSTOM_PATH_TO_ELM_FORMAT=elm-format
prettier --write "**/*.elm"
export PRETTIER_PLUGIN_ELM_CUSTOM_PATH_TO_ELM_FORMAT=/path/to/non-global/elm-format
prettier --write "**/*.elm"

I'm happy to consider a PR that introduces this. It'd be great if you could add test coverage to the change. Upgrading dependencies could be also worth it given the opportunity.


UPD: Looks like cache must be made aware of the value too.

@krksgbr
Copy link
Author

krksgbr commented Apr 18, 2019

Awesome:)
Will try and send you a PR today.

@kachkaev
Copy link
Member

Cool. I won't be able to release at least for another week and a half, so no rush 😉

@krksgbr
Copy link
Author

krksgbr commented Apr 20, 2019

Hey @kachkaev!

In the meantime, I've had a discussion about this with @knuton. After a bit more careful consideration we came to the conclusion that the implementation proposed above would not be as useful for us as I initially thought.

Two issues we have with it are:

  • It would lead to surprising behavior if one doesn't know about having to set the environment variable.
  • We're transitioning to the use of Lorri (nix-shell replacement for project development), and in the short-term, it will become a bit of a hassle to set environment variables involving nix paths because some functionality is still missing in Lorri.

What do you think of giving preference to the global installation of elm-format and falling back to the one installed by the plugin when it is not found? I feel like it would be closer to the behavior specified in the elm-format plugin development guidelines.

Specifically:

If the path to elm-format has not [been] explicitly specified by the user, the plugin should automatically find elm-format if it is located on the $PATH or in /usr/local/bin/

@kachkaev
Copy link
Member

I see. Let’s then close this issue and reopen it if you have further thoughts.

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

No branches or pull requests

2 participants