Add escape sequences and test #1701
Conversation
|
They are in fact needed; see #1322 (comment), #1279, and #1278. |
|
We should add a regression test that aliases |
|
I updated my pull request to only remove the backslashes in README. I also added a test, in which an alias for |
|
|
||
| alias .='cat' | ||
|
|
||
| \. ../../nvm.sh |
There was a problem hiding this comment.
the \. here will make the alias not do anything; we don't need to test . vs \., what we need to test is the install script (not sourcing).
There was a problem hiding this comment.
So I have to define the alias in all files under test/install_script/, so that install.sh would always fail, if . is not escaped?
There was a problem hiding this comment.
not necessarily in all files, a single test would be better.
README.md
Outdated
| @@ -135,8 +135,8 @@ Now add these lines to your `~/.bashrc`, `~/.profile`, or `~/.zshrc` file to hav | |||
|
|
|||
| ```sh | |||
| export NVM_DIR="$HOME/.nvm" | |||
| [ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" # This loads nvm | |||
| [ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion" # This loads nvm bash_completion | |||
| [ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" # This loads nvm | |||
There was a problem hiding this comment.
People will copy-paste this code; it's important it retains the \.
There was a problem hiding this comment.
But there are lines in the README, where no \ has been placed (see section “Install script”). These lines have to be modified, too.
There was a problem hiding this comment.
ah, that's an oversight then. anything in a code block should have the \.
|
I’ve updated my pull request. Do I need to add the test file to something? The new test is not executed on Travis. |
ihmels
changed the title
|
Yes, you'll need to |
|
I made the test executable and fixed another bug that caused the alias not to be expanded. |
| alias .='cat' | ||
| NVM_ENV=testing \. ../../install.sh | ||
| \. ../../install.sh > /dev/null |
There was a problem hiding this comment.
the NVM_ENV=testing is still required tho; otherwise it will actually invoke the install script.
There was a problem hiding this comment.
But when I set NVM_ENV=testing, nvm_do_install will not be executed, so I can’t test the sourcing.
There was a problem hiding this comment.
You can test the sourcing without executing that function; for example, type nvm_do_install will only exit zero if it was sourced correctly.
There was a problem hiding this comment.
I want to test line 368 in install.sh. type nvm_do_install is not enough or am I wrong?
There was a problem hiding this comment.
ah, true. but in that case, you can invoke nvm_do_install manually.
I think, the backslashes are not needed.