[Refactor] minor "if" and "return" improvement #1486
Conversation
nvm.sh
Outdated
| setopt nomatch | ||
| fi | ||
| return 2 | ||
| if [ -d "$FILE" ] && ! nvm_check_file_permissions "$FILE"; then |
There was a problem hiding this comment.
this isn't the same; if the file is a directory but nvm_check_file_permissions returns true, i don't want anything to happen. With this change, it ends up hitting the else branch.
There was a problem hiding this comment.
Really, I think here is no else branch?
There was a problem hiding this comment.
aha, sorry, you're right, there's no else.
However, in the original code, if the file is a directory, nothing happened outside of the nvm_check_file_permissions branch - in your new code, if the file is a directory but the permissions check fails, it falls to the elif - which checks that it exists, and is not writable and not owned (possible for a directory), which means the file-related error message would be printed.
Am I reading this incorrectly?
There was a problem hiding this comment.
You're correct, reverted this part, thanks.
nvm.sh
Outdated
| fi | ||
|
|
||
| if [ -n "${NPM_CONFIG_PREFIX-}" ] && ! (nvm_tree_contains_path "$NVM_DIR" "$NPM_CONFIG_PREFIX" >/dev/null 2>&1); then | ||
| elif [ -n "${NPM_CONFIG_PREFIX-}" ] && ! (nvm_tree_contains_path "$NVM_DIR" "$NPM_CONFIG_PREFIX" >/dev/null 2>&1); then |
There was a problem hiding this comment.
I don't think we need all these elifs; I actually prefer the separate if blocks here.
There was a problem hiding this comment.
ok, reverted both above
PeterDaveHello
force-pushed
the
refactor
branch
2 times, most recently
from
a10a5ef to
315f0df
Compare
No description provided.