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.

Sign up for GitHub

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

Jump to bottom

feat: auto invoke nvm use on PWD change #181

Closed
wants to merge 6 commits into from
Closed

feat: auto invoke nvm use on PWD change #181

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
611d1f1
feat: auto invoke nvm use on PWD change
edouard-lopez Apr 14, 2022
dc66fca
feat: hide behind a feature flag
edouard-lopez Apr 14, 2022
dc7764e
refactor: rename to `nvm_use_on_pwd_change`
edouard-lopez Apr 15, 2022
0eb0f40
refactor: condition to $nvm_use_on_pwd_change truthiness
edouard-lopez Apr 15, 2022
fe78ecf
docs: update
edouard-lopez Apr 15, 2022
60fe6de
typo
edouard-lopez May 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,11 @@ Then run `nvm install` to install or `nvm use` to activate that version. Works f
nvm install
```

Automatic activation can be enable with `set --universal auto_invoke_nvm true`

edouard-lopez marked this conversation as resolved.
Show resolved Hide resolved
## `$nvm_mirror`

If you would like to use a different Node.js mirror that has the same layout as the default at https://nodejs.org/dist, you can set `$nvm_mirror`. A typical example is users from China using:
If you would like to use a different Node.js mirror that has the same layout as the default at <https://nodejs.org/dist>, you can set `$nvm_mirror`. A typical example is users from China using:
edouard-lopez marked this conversation as resolved.
Show resolved Hide resolved

```console
set --universal nvm_mirror https://npm.taobao.org/mirrors/node
Expand Down
8 changes: 8 additions & 0 deletions functions/_nvm_auto_invoke.fish
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
function _nvm_auto_invoke \
edouard-lopez marked this conversation as resolved.
Show resolved Hide resolved
--on-variable PWD \
--description 'Use Node.js version specified by project automatically'

if set --query auto_invoke_nvm; and test "$auto_invoke_nvm" = true
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather have the flag prevent _nvm_auto_invoke from being defined at all, rather than define the --on-variable PWD hooks and incur this check every time.

Copy link
Author

@edouard-lopez edouard-lopez Apr 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing something like below break the autoloading mechanism:

if set --query nvm_use_on_pwd_change; and test "$nvm_use_on_pwd_change" = true
    function _nvm_use_on_pwd_change \
        --on-variable PWD \
…
    end
end

Should I hook _nvm_use_on_pwd_chang on both the flag variable ($nvm_use_on_pwd_change) AND $PWD? Or do something like https://github.com/jorgebucaran/hydro/blob/d4875065ceea226f58ead97dd9b2417937344d6e/conf.d/hydro.fish#L119-L120 ?

nvm use 2>/tmp/_nvm_auto_invoke.log
end
end
48 changes: 48 additions & 0 deletions tests/_nvm_auto_invoke.test.fish
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
source (status dirname)/../functions/_nvm_auto_invoke.fish

function clean_up
echo "" >/tmp/_nvm_auto_invoke.log
rm --force .nvmrc
set --erase --universal auto_invoke_nvm
end

function activate_feature
set --universal auto_invoke_nvm true
end

@test "When feature-flag is set to default then auto-invoke is skipped" (
clean_up
set --erase --universal auto_invoke_nvm

_nvm_auto_invoke

cat /tmp/_nvm_auto_invoke.log
) = ''

@test "When feature-flag is enable then auto-invoke is called" (
clean_up
activate_feature

_nvm_auto_invoke

cat /tmp/_nvm_auto_invoke.log
) = 'nvm: Invalid version or missing ".nvmrc" file'

@test "auto-invoke changes Node version when .nvmrc file exists with value" (
clean_up
activate_feature
echo "invalid version" > .nvmrc

_nvm_auto_invoke

cat /tmp/_nvm_auto_invoke.log
) = 'nvm: Node version not installed or invalid: ""'

@test "auto-invoke ignored when .nvmrc file is missing" (
clean_up
activate_feature

_nvm_auto_invoke

cat /tmp/_nvm_auto_invoke.log
) = 'nvm: Invalid version or missing ".nvmrc" file'