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

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

Open
avh4 opened this issue Oct 7, 2018 · 6 comments · May be fixed by #653
Open

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

avh4 opened this issue Oct 7, 2018 · 6 comments · May be fixed by #653

Comments

@avh4
Copy link
Owner

avh4 commented Oct 7, 2018

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

@lassik
Copy link

lassik commented Oct 7, 2018

See also: #523, #545

@lassik
Copy link

lassik commented Oct 7, 2018

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
Copy link

lassik commented Oct 7, 2018

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

@lassik
Copy link

lassik commented Oct 7, 2018

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).

@ymtszw
Copy link

ymtszw commented Oct 12, 2018

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.

rlefevre added a commit to rlefevre/elm-format that referenced this issue Nov 25, 2019
@rlefevre rlefevre linked a pull request Nov 25, 2019 that will close this issue
rlefevre added a commit to rlefevre/elm-format that referenced this issue Nov 25, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Nov 25, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Nov 25, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Nov 25, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Nov 25, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Nov 26, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Nov 26, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Dec 2, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Dec 2, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Dec 2, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Dec 2, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Dec 2, 2019
rlefevre added a commit to rlefevre/elm-format that referenced this issue Dec 2, 2019
@avh4 avh4 added the 0.9 (temporary label for search filtering) label Sep 24, 2020
@avh4
Copy link
Owner Author

avh4 commented Dec 11, 2021

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 avh4 removed the 0.9 (temporary label for search filtering) label Feb 20, 2023
@avh4 avh4 removed this from the 0.9.0-exp next experimental milestone Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants