Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
ls -L (follow symlinks) #665
Conversation
|
@nfischer Perhaps the difference I pointed out only happens in my local environment? Tests pass: Not sure how to proceed |
| @@ -60,7 +62,7 @@ function _ls(options, paths) { | |||
| relName = relName.replace(/\\/g, '/'); | |||
| } | |||
| if (options.long) { | |||
| stat = stat || fs.lstatSync(abs); | |||
| stat = stat || options.link ? fs.statSync(abs) : fs.lstatSync(abs); | |||
freitagbr
Feb 23, 2017
Contributor
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 |
|
@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.