avh4 / elm-upgrade

Upgrade Elm projects
https://www.npmjs.com/package/elm-upgrade
MIT License
122 stars 15 forks source link

Elm upgrade placing broken version number in elm.json #94

Open Muzz opened 3 years ago

Muzz commented 3 years ago

Finally getting around to seeing if i can update an old project of mine. And for some reason the elm.json that is getting generated has a carriage return in it. This is on windows 10 with the latest elm-format, and elm, with the latest elm-upgrade.

Assistance would be appreciated.

`C:\GITHUB\PrismaPalette\gamut-commander\html>npx elm-upgrade@latest INFO: Found elm at node_modules.bin\elm.CMD INFO: Found elm 0.19.1 INFO: Found elm-format at node_modules.bin\elm-format.CMD INFO: Found elm-format 0.8.5 INFO: Cleaning ./elm-stuff before upgrading INFO: Converting elm-package.json -> elm.json INFO: Detected an application project (this project has no exposed modules) WARNING: MichaelCombs28/elm-mdl has not been upgraded to 0.19 yet! INFO: Installing latest version of debois/elm-dom -- PROBLEM WITH VERSION ----------------------------------------------- elm.json

I got stuck while reading your elm.json file. I was expecting a version number here:

8| "elm-version": "0.19.1\r", ^ I need something like "1.0.0" or "2.0.4" that explicitly states all three numbers!

WARNING: Failed to upgrade debois/elm-dom! INFO: Switching from elm-community/linear-algebra (deprecated) to elm-explorations/linear-algebra INFO: Installing latest version of elm-explorations/linear-algebra`

avh4 commented 3 years ago

Huh, that's interesting. I'm guessing it's due to nodejs automatic newline conversion? I'd think it'd have to be this line where things go wrong https://github.com/avh4/elm-upgrade/blob/master/upgrade.js#L130 Does fs.writeFileSync and/or JSON.stringify insert \r on Windows? If we could update that function to not do that, I'd think that should fix things.

avh4 commented 3 years ago

Ah, I realized what I just wrote is probably wrong. It's more likely that the problem is here https://github.com/avh4/elm-upgrade/blob/master/upgrade.js#L175 we need to strip out the \r when parsing the version number from elm --version.

Muzz commented 3 years ago

Thanks for looking into that so quickly!