-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
refactor: read alias files with shell builtins instead of sed/awk/head/tail pipeline #3787
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1323,7 +1323,21 @@ nvm_alias() { | |
| return 2 | ||
| fi | ||
|
|
||
| command sed 's/#.*//; s/[[:space:]]*$//' "${NVM_ALIAS_PATH}" | command awk 'NF' | ||
| local NVM_ALIAS_LINE | ||
| while IFS= read -r NVM_ALIAS_LINE || [ -n "${NVM_ALIAS_LINE}" ]; do | ||
| NVM_ALIAS_LINE="${NVM_ALIAS_LINE%%#*}" | ||
| case "${NVM_ALIAS_LINE}" in | ||
| *[![:space:]]*) ;; | ||
| *) continue ;; | ||
| esac | ||
| while : ; do | ||
| case "${NVM_ALIAS_LINE}" in | ||
| *[[:space:]]) NVM_ALIAS_LINE="${NVM_ALIAS_LINE%[[:space:]]}" ;; | ||
| *) break ;; | ||
| esac | ||
| done | ||
| nvm_echo "${NVM_ALIAS_LINE}" | ||
| done < "${NVM_ALIAS_PATH}" | ||
| } | ||
|
|
||
| nvm_ls_current() { | ||
|
|
@@ -1356,13 +1370,14 @@ nvm_resolve_alias() { | |
| local ALIAS | ||
| ALIAS="${PATTERN}" | ||
| local ALIAS_TEMP | ||
| local ALIAS_OUTPUT | ||
|
|
||
| local SEEN_ALIASES | ||
| SEEN_ALIASES="${ALIAS}" | ||
| local NVM_ALIAS_INDEX | ||
| NVM_ALIAS_INDEX=1 | ||
| SEEN_ALIASES=" ${ALIAS} " | ||
| while true; do | ||
| ALIAS_TEMP="$( (nvm_alias "${ALIAS}" 2>/dev/null | command head -n "${NVM_ALIAS_INDEX}" | command tail -n 1) || nvm_echo)" | ||
| ALIAS_OUTPUT="$(nvm_alias "${ALIAS}" 2>/dev/null)" || ALIAS_OUTPUT='' | ||
| ALIAS_TEMP="${ALIAS_OUTPUT%% | ||
| *}" | ||
|
|
||
| if [ -z "${ALIAS_TEMP}" ]; then | ||
| break | ||
|
|
@@ -1373,7 +1388,7 @@ nvm_resolve_alias() { | |
| break | ||
| fi | ||
|
|
||
| SEEN_ALIASES="${SEEN_ALIASES}\\n${ALIAS_TEMP}" | ||
| SEEN_ALIASES="${SEEN_ALIASES}${ALIAS_TEMP} " | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: cycle detection is broken by this format change.
if command printf '%b' "${SEEN_ALIASES}" | nvm_grep -q -e "^${ALIAS_TEMP}$"; then
Reproduces against the existing Suggested fix (matches what the PR description claims was done): case "${SEEN_ALIASES}" in
*" ${ALIAS_TEMP} "*) ALIAS="∞"; break ;;
esacThe surrounding spaces in |
||
| ALIAS="${ALIAS_TEMP}" | ||
| done | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an alias file whose name contains spaces | ||
| mkdir -p ../../../alias | ||
| printf 'v22.1.0\n' > "../../../alias/test edge spaces" | ||
| ACTUAL="$(nvm_alias "test edge spaces")" | ||
| EXPECTED='v22.1.0' | ||
| [ "${ACTUAL}" = "${EXPECTED}" ] || die "expected >${EXPECTED}<, got >${ACTUAL}<" | ||
|
|
||
| rm -f "../../../alias/test edge spaces" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an alias file where the first line has a valid version | ||
| # followed by a second line of binary-like data | ||
| printf 'v22.1.0\n' > ../../../alias/test-edge-binary | ||
| printf '\001\002\003\377\n' >> ../../../alias/test-edge-binary | ||
| ACTUAL="$(nvm_alias test-edge-binary)" | ||
| # nvm_alias emits every non-blank, non-comment line — first line should be the version | ||
| FIRST_LINE="$(nvm_echo "${ACTUAL}" | command head -n 1)" | ||
| EXPECTED='v22.1.0' | ||
| [ "${FIRST_LINE}" = "${EXPECTED}" ] || die "expected first line >${EXPECTED}<, got >${FIRST_LINE}<" | ||
|
|
||
| rm -f ../../../alias/test-edge-binary |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an alias file with Windows-style line endings (CRLF) | ||
| printf 'v22.1.0\r\n' > ../../../alias/test-edge-cr | ||
| ACTUAL="$(nvm_alias test-edge-cr)" | ||
| EXPECTED='v22.1.0' | ||
| [ "${ACTUAL}" = "${EXPECTED}" ] || die "expected >${EXPECTED}<, got >${ACTUAL}<" | ||
|
|
||
| # Create an alias file with bare carriage return (no newline) | ||
| printf 'v22.2.0\r' > ../../../alias/test-edge-cr-bare | ||
| ACTUAL="$(nvm_alias test-edge-cr-bare)" | ||
| EXPECTED='v22.2.0' | ||
| [ "${ACTUAL}" = "${EXPECTED}" ] || die "expected >${EXPECTED}< for bare CR, got >${ACTUAL}<" | ||
|
|
||
| rm -f ../../../alias/test-edge-cr ../../../alias/test-edge-cr-bare |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an alias file containing only comments | ||
| printf '# this is a comment\n# another comment\n' > ../../../alias/test-edge-comments | ||
| ACTUAL="$(nvm_alias test-edge-comments 2>/dev/null)" | ||
| EXIT_CODE="$(nvm_alias test-edge-comments 2>/dev/null; echo $?)" | ||
| [ -z "${ACTUAL}" ] || die "expected empty output for comment-only alias file, got >${ACTUAL}<" | ||
| [ "${EXIT_CODE}" = '0' ] || die "expected exit code 0, got ${EXIT_CODE}" | ||
|
|
||
| rm -f ../../../alias/test-edge-comments |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an alias file with an embedded NUL byte after the version. | ||
| # NUL handling varies by shell: some stop at NUL, others read through it. | ||
| # The function must not crash, and must emit output starting with the version. | ||
| printf 'v22.1.0\000garbage\n' > ../../../alias/test-edge-nul | ||
| ACTUAL="$(nvm_alias test-edge-nul)" | ||
| EXIT_CODE=$? | ||
| [ "${EXIT_CODE}" = '0' ] || die "expected exit code 0, got ${EXIT_CODE}" | ||
| case "${ACTUAL}" in | ||
| v22.1.0*) ;; # OK — starts with the version regardless of NUL handling | ||
| *) die "expected output starting with >v22.1.0<, got >${ACTUAL}<" ;; | ||
| esac | ||
|
|
||
| rm -f ../../../alias/test-edge-nul |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an empty alias file | ||
| printf '' > ../../../alias/test-edge-empty | ||
| ACTUAL="$(nvm_alias test-edge-empty 2>/dev/null)" | ||
| EXIT_CODE="$(nvm_alias test-edge-empty 2>/dev/null; echo $?)" | ||
| [ -z "${ACTUAL}" ] || die "expected empty output for empty alias file, got >${ACTUAL}<" | ||
| [ "${EXIT_CODE}" = '0' ] || die "expected exit code 0, got ${EXIT_CODE}" | ||
|
|
||
| rm -f ../../../alias/test-edge-empty |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an alias file with trailing form feed | ||
| printf 'v22.1.0\f\n' > ../../../alias/test-edge-ff | ||
| ACTUAL="$(nvm_alias test-edge-ff)" | ||
| EXPECTED='v22.1.0' | ||
| [ "${ACTUAL}" = "${EXPECTED}" ] || die "expected >${EXPECTED}< for form feed, got >${ACTUAL}<" | ||
|
|
||
| # Create an alias file with trailing vertical tab | ||
| printf 'v22.2.0\v\n' > ../../../alias/test-edge-vt | ||
| ACTUAL="$(nvm_alias test-edge-vt)" | ||
| EXPECTED='v22.2.0' | ||
| [ "${ACTUAL}" = "${EXPECTED}" ] || die "expected >${EXPECTED}< for vertical tab, got >${ACTUAL}<" | ||
|
|
||
| rm -f ../../../alias/test-edge-ff ../../../alias/test-edge-vt |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an alias file with no trailing newline | ||
| printf 'v22.1.0' > ../../../alias/test-edge-no-newline | ||
| ACTUAL="$(nvm_alias test-edge-no-newline)" | ||
| EXPECTED='v22.1.0' | ||
| [ "${ACTUAL}" = "${EXPECTED}" ] || die "expected >${EXPECTED}<, got >${ACTUAL}<" | ||
|
|
||
| rm -f ../../../alias/test-edge-no-newline |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an alias file with a very long line (version followed by a long comment) | ||
| LONG_COMMENT="$(printf '%0*d' 10000 0 | command tr '0' 'x')" | ||
| printf 'v22.1.0 #%s\n' "${LONG_COMMENT}" > ../../../alias/test-edge-long | ||
| ACTUAL="$(nvm_alias test-edge-long)" | ||
| EXPECTED='v22.1.0' | ||
| [ "${ACTUAL}" = "${EXPECTED}" ] || die "expected >${EXPECTED}<, got >${ACTUAL}<" | ||
|
|
||
| rm -f ../../../alias/test-edge-long |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an alias file with trailing spaces and tabs | ||
| printf '22.1.0 \t \n' > ../../../alias/test-edge-trailing-ws | ||
| ACTUAL="$(nvm_alias test-edge-trailing-ws)" | ||
| EXPECTED='22.1.0' | ||
| [ "${ACTUAL}" = "${EXPECTED}" ] || die "expected >${EXPECTED}<, got >${ACTUAL}<" | ||
|
|
||
| rm -f ../../../alias/test-edge-trailing-ws |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create a 4-deep alias chain: hop1 -> hop2 -> hop3 -> hop4 -> 0.0.99 | ||
| echo 'hop2' > ../../../alias/hop1 | ||
| echo 'hop3' > ../../../alias/hop2 | ||
| echo 'hop4' > ../../../alias/hop3 | ||
| echo '0.0.99' > ../../../alias/hop4 | ||
|
|
||
| ACTUAL="$(nvm_resolve_alias hop1)" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth adding a sibling test for cycles (1-hop self-reference and a multi-hop loop). The existing |
||
| EXPECTED='v0.0.99' | ||
| [ "${ACTUAL}" = "${EXPECTED}" ] || die "expected >${EXPECTED}< for 4-deep chain, got >${ACTUAL}<" | ||
|
|
||
| rm -f ../../../alias/hop1 ../../../alias/hop2 ../../../alias/hop3 ../../../alias/hop4 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| #!/bin/sh | ||
|
|
||
| die () { echo "$@" ; exit 1; } | ||
|
|
||
| export NVM_DIR="$(cd ../../.. && pwd)" | ||
|
|
||
| : nvm.sh | ||
| \. "${NVM_DIR}/nvm.sh" | ||
|
|
||
| # Create an alias pointing to a version that does not exist as an alias | ||
| echo '99.99.99' > ../../../alias/test-edge-noexist | ||
|
|
||
| ACTUAL="$(nvm_resolve_alias test-edge-noexist)" | ||
| EXPECTED='v99.99.99' | ||
| EXIT_CODE="$(nvm_resolve_alias test-edge-noexist >/dev/null 2>&1; echo $?)" | ||
| [ "${ACTUAL}" = "${EXPECTED}" ] || die "expected >${EXPECTED}<, got >${ACTUAL}<" | ||
| [ "${EXIT_CODE}" = '0' ] || die "expected exit code 0, got ${EXIT_CODE}" | ||
|
|
||
| rm -f ../../../alias/test-edge-noexist |
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.
Stripping one whitespace char per iteration is O(n²) for trailing whitespace. Not a real concern for alias files in practice, but a single-pass equivalent (e.g.
NVM_ALIAS_LINE="${NVM_ALIAS_LINE%"${NVM_ALIAS_LINE##*[![:space:]]}"}") avoids the loop entirely.