Add symlink support (Fixes #42) #49
Conversation
| - On write the stream will create a symbolic link (i.e. symlink) on disk at the folder/cwd specified. | ||
| - After creating the symbolic link, it will be emitted from the stream so you can keep piping these around. | ||
| - The file will be modified after being written to this stream: | ||
| - `cwd`, `base`, and `path` will be overwritten to match the folder |
There was a problem hiding this comment.
you read in some files, symlink, then symlink again. you have the original files, a pointer to the original files, and a pointer to the pointer to the original files. does that sound right?
There was a problem hiding this comment.
I think so. Basically a vinyl file object is read in from the stream, a symlink is created pointing to it on disk, then the vinyl object is altered to point to the symlink and it's written back to the stream.
I thought this behaviour would be the most similar to dest's behaviour.
There was a problem hiding this comment.
@JonAbrams Yeah, this is how dest works just curious if this seems right for symlinks which is a lot differnet.
@sindresorhus @phated thoughts?
There was a problem hiding this comment.
I had actually first had it return the original files, untouched, after the symlinks were created, but considering the two possibilities, either return the original file or the new symlink, the latter seemed to behave more like dest (since dest returns the newly written file) which would be "least surprising" for the user.
There was a problem hiding this comment.
@JonAbrams Fair enough, I think we can keep this behavior
1 similar comment
| var mkdirp = require('mkdirp'); | ||
| var fs = require('graceful-fs'); | ||
|
|
||
| exports.prepDestAndResponse = function (outFolder, file, opt, cb) { |
There was a problem hiding this comment.
Not a fan of lib/common - would much rather just have lib/prepareWrite.js which exports a function.
| var through2 = require('through2'); | ||
| var mkdirp = require('mkdirp'); | ||
| var fs = require('graceful-fs'); | ||
| var prewrite = require('../prepareWrite'); |
There was a problem hiding this comment.
var name and file name should be the same
There was a problem hiding this comment.
Do you prefer prewrite or prepareWrite? I'd go with prepareWrite if it's all the same.
There was a problem hiding this comment.
@JonAbrams prepareWrite is fine
| var mkdirp = require('mkdirp'); | ||
| var fs = require('graceful-fs'); | ||
|
|
||
| module.exports = function (outFolder, file, opt, cb) { |
There was a problem hiding this comment.
Other support functions in lib/dest don't have unit tests. I thought since the code coverage didn't change (and no new functionality was added) extra tests wouldn't be needed. The existing feature tests in test/dest.js and test/symlink.js seem to cover it just fine too.
If you still think it's required I can whip something up tomorrow.
There was a problem hiding this comment.
@JonAbrams I can write some tests for this. Usually any function that had to be split into it's own file I'll do tests for. Smaller support functions within a file don't need specific testing IMO.
Add symlink support (Fixes #42)
Add symlink support (Fixes #42)
It's missing the option for relative symlinking, but this should hopefully do the trick for now. Merry Xmas!