fix: convert error output to be consistent cross-platform #684
Conversation
Codecov Report
@@ Coverage Diff @@
## master #684 +/- ##
==========================================
+ Coverage 94.3% 94.32% +0.01%
==========================================
Files 33 33
Lines 1176 1180 +4
==========================================
+ Hits 1109 1113 +4
Misses 67 67
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I have mixed feelings about this. This is a definitely a breaking change; there might be systems out there that depend on the error output to have backslashes. It would be better to just sanitize the paths themselves, but this would be complicated to implement.
Why would that be better? You mean to change For what it's worth, our docs don't actually specify what these error messages may be. We should decide if we really want to consider these part of our public API. In fact, The only real motivation we have for this right now is to land tests, which probably isn't enough to justify something on the fence like this. So feel free to push the milestone back to v0.8. |
|
I think you've convinced me. In the error messages, \ characters should only occur in the paths. And because the error messages are not documented, there should be no reason to rely on them. This looks good so I'll go ahead and merge it. |
|
Er, there are merge conflicts. |
The error output produced by `shell.error()` or `result.stderr` should not be inconsistent between platforms. This ensures that path separators are always printed by ShellJS as `/` instead of as `\` on Windows. This should allow scripts using ShellJS to be more consistent cross-platform. We were not previously relying on error output to always be consistent-- only checking its truthiness. Since this was not part of our tested API, it should be reasonable to change this and release in a patch. Fixes #681
nfischer
force-pushed
the
fix-consistent-error-output
branch
from
c90113d
to
3940359
Compare
|
Rebased off master. Will land this tonight. |
|
I also fixed a TODO which depended on this being landed. |
The error output produced by
shell.error()orresult.stderrshouldnot be inconsistent between platforms. This ensures that path separators
are always printed by ShellJS as
/instead of as\on Windows. Thisshould allow scripts using ShellJS to be more consistent cross-platform.
We had only one test case which relied on the old behavior, and it's in
pushd, which we don't any issues for. Since it's only the error outputthat's changed, it's probably a reasonable fix to make.
Fixes #681