fix #268: Revert "fix #246: remove any double quotes or single quotes… #278
Conversation
…gle quotes from os.tmpdir also sanitize dir option, the template option and the name option" This reverts commit c8823e5. Single quotes must not be removed from paths because they are valid (even if hard to use) on all OSes. Double quotes are only disallowed on Windows, but tmp should not change any arg it gets; instead, it should rely on the underlying fs API to fail with an error that the user needs to fix.
|
I successfully verified in my project that reverting this commit fixes the regression introduced when we upgraded from 0.1.0 to 0.2.1. |
|
Thank you for the PR. I will review the changes and then merge as this is a long standing issue. I am still wondering, though, why no CI checks have been executed. |
| @@ -53,35 +50,6 @@ describe('tmp', function () { | |||
| } | |||
| }); | |||
| }); | |||
| describe('on issue #246', function () { | |||
There was a problem hiding this comment.
Could you leave these tests and make these so that they check for the presence of the single quotes and double quotes instead?
There was a problem hiding this comment.
Absolutely! I wasn't sure which you would have preferred between simply reverting or keeping the tests but amended to serve as regression tests. I'll do this when I can, either tonight or tomorrow.
There was a problem hiding this comment.
Please see a831713.
| @@ -63,39 +62,6 @@ describe('tmp', function () { | |||
| }); | |||
| }); | |||
| }); | |||
| describe('on issue #246', function () { | |||
There was a problem hiding this comment.
Same here, tests must stay in place, yet test for the presence of the quotes.
There was a problem hiding this comment.
Please see a831713.
|
@raszi Would you be so kind to enable github actions so that we can use that for the CI instead of appveyor and travis-ci? |
|
@raszi Would you be so kind to enable github projects so that we can visually manage issues in projects? That would be great! |
Enabled. |
|
@raszi Thank you. I just looked into the cost for the github actions, I think that this is a no-go 😄 |
|
Yet, Github Free includes 500GB and 2000 Minutes... |
Yes, I believe it could work for this project, I am using that with other free projects. |
|
@raszi I do not see the projects, yet. |
… from os.tmpdir also sanitize dir option, the template option and the name option"
This reverts commit c8823e5.
Single quotes must not be removed from paths because they are valid (even if hard to use)
on all OSes. Double quotes are only disallowed on Windows, but tmp should not change any
arg it gets; instead, it should rely on the underlying fs API to fail with an error
that the user needs to fix.