elm-format icon indicating copy to clipboard operation
elm-format copied to clipboard

auto-detect elm version per file, instead of based on cwd

Open avh4 opened this issue 5 years ago • 6 comments

In 0.8.1, the elm-version is auto-detected based on the presence of elm.json or elm-package.json in the current working directory.

Proposal: instead of checking the current working directory, for each file check the file's folder and if not found traverse upwards until a package file is found.

It seems like this would be more flexible, and still do the right thing in all current cases? I think this would also allow plugins to not have to pass --elm-version unless they are using --stdin

avh4 avatar Oct 07 '18 03:10 avh4

See also: #523, #545

lassik avatar Oct 07 '18 07:10 lassik

I think it would be the right thing to traverse directories upwards, and I'm under the impression that this is what Git and many other tools do too. But not 100% confident about all edge cases that may come up. I'd say, ask a few very experienced programmers if there are any problems with it, and if none of them come up with anything, just do it 😄

lassik avatar Oct 07 '18 07:10 lassik

(Moved comments about --stdin to a separate issue, #563)

lassik avatar Oct 07 '18 08:10 lassik

Doing the version check separately for each file given on the command line would definitely be the right thing, since they may (in principle, at least) be from different Elm projects.

And to make sure that --elm-version can be given only once so people don't think they can do things like elm-format --elm-version 0.18 foo.elm --elm-version 0.19 bar.elm. The check currently seems to be in place (the error message is a bit misleading, Invalid option '--elm-version', but that's probably ok).

lassik avatar Oct 07 '18 08:10 lassik

Love this. I have done the same thing for my small Atom plugin that runs elm-analyse back in 0.18. https://github.com/ymtszw/linter-elm-analyse/blob/master/index.js#L58 In my case it was (1) traverse directories upward until it finds elm-package.json, (2) if found, set CWD there, (3) otherwise bail out.

ymtszw avatar Oct 12 '18 20:10 ymtszw

I haven't heard anyone bring this up in a long time, I'm guessing because very few folks have both 0.18 and 0.19 projects anymore? If anyone thinks this would still be useful, please chime in.

avh4 avatar Dec 11 '21 05:12 avh4