-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
[Fix] Improve .nvmrc reading process #1740
Conversation
Fixes nvm-sh#1015. Fixes nvm-sh#1712.
nvm.sh
Outdated
@@ -293,7 +293,7 @@ nvm_rc_version() { | |||
nvm_err "No .nvmrc file found" | |||
return 1 | |||
fi | |||
read -r NVM_RC_VERSION < "${NVMRC_PATH}" || printf '' | |||
NVM_RC_VERSION="$(command head -n 1 "${NVMRC_PATH}" | command tr -d '\r')" || printf '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
bba70f5
to
7984305
Compare
\. ../../nvm.sh | ||
|
||
# normal .nvmrc | ||
printf '0.999.0\n' > .nvmrc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also add an example that has no newline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
7984305
to
787bbe3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, but it's a pretty important part of the code to change, so I'm going to let it bake for a bit before merging, and test it locally.
Great, let's hope to get it done soon! |
I tested this out with both linux new lines and windows new lines and it seemed to have worked! |
I hope > 1.5 months bake is good enough now :) |
787bbe3
to
e7b4219
Compare
Improve how we read the version in
.nvmrc
to avoid the impact ofCR
/CRLF
as newline char.Fixes #1015. Fixes #1712.