Conversation
|
@nfischer Perhaps the difference I pointed out only happens in my local environment? Tests pass: Not sure how to proceed 😅 |
src/ls.js
Outdated
| } | ||
| if (options.long) { | ||
| stat = stat || fs.lstatSync(abs); | ||
| stat = stat || options.link ? fs.statSync(abs) : fs.lstatSync(abs); |
There was a problem hiding this comment.
I think you need to put the ternary in parentheses:
stat = stat || (options.link ? fs.statSync(abs) : fs.lstatSync(abs));Otherwise the condition for the ternary is stat || options.link.
|
@freitagbr @nfischer Appveyor is using a Windows environment, now I understand the difference 😅
I just reported it with a screenshot on #666 |
|
@frankdiox this is a relatively small change that affects a lot of test cases. Do you think there's any way we could make this affect fewer test cases? What if we do |
|
That change would fix most of the breaking test cases on Windows. I would expect we'll still have one breaking test case on Windows, because it handles symlnks differently than unix does. If that's the case, would it make sense to skip this test only for Windows? Here's an example of how we test unix-only code. |
This reverts commit dbb057a.
|
@nfischer Yep, I didn't see there was another symlink in |
nfischer
left a comment
There was a problem hiding this comment.
@frankdiox Thanks for the hard work! This LGTM
|
Deferring to @freitagbr for final approval |
|
LGTM too. |
This closes #563
I added some tests for
-Loption and fixed the other.@nfischer If I'm not mistaken, there is a difference between bash's
ls -Rand shelljs's related to dir symlinks. Bash only shows the symlink itself but shelljs shows the direct children as well (1 level). In this case,ls -RLwill show all the levels, not only the first one.By the way, I think this test is a mistake since it is just a copy of another one, so I removed it.