feat(clone): option to not clone buffer, support custom properties #16
Conversation
option `{ contents: false }` causes the clone to reference-copy buffer contents.
Custom properties that exist in the original file are deep-cloned to the new File.
| var clone = new File(); | ||
|
|
||
| Object.keys(this).forEach(function(key) { | ||
| if (key !== '_contents' && key !== 'stat' && this.hasOwnProperty(key)) { |
There was a problem hiding this comment.
hasOwnProperty is unnecessary as Object.keys() doesn't include properties of the prototype chain.
There was a problem hiding this comment.
hasOwnProperty is redundant, where does _contents come from? That isn't a standard attribute
|
Sorry for letting this sit for so long, I was traveling. I will look at this now |
| clone.contents = this.contents; | ||
| } | ||
|
|
||
| clone.stat = this.stat ? cloneStats(this.stat) : null; |
There was a problem hiding this comment.
Shouldn't all attributes follow the same code path? If opt[k] === false then reference, else clone?
|
How about this PR? I want this feature. |
|
In the woods with bad internet connection right now. I will review when I'm back in town |
|
All right, have a good travel— On Sat, Jul 5, 2014 at 1:31 PM, Eric Schoffstall notifications@github.com
|
|
@contra Are you coming back, do you have time to review this pr? |
|
I never got a response to the code comments I left :/ |
|
Or you can review my pr #26 as you like, I just want to support this feature. |
|
How about a PR for just the custom properties feature? Because the holdup here is about reference-copying and @stu-salsbury not responding. |
|
I'll take a wholesome PR - code style matches, tests, etc. any day and merge it quickly as long as it looks good @vweevers Send that PR |
|
Good, thx @vweevers . If you have no time, I will update my PR. |
|
deep clone has been implemented, need |
|
@popomore Yep - btw could you try to use a lower level clone (like the node-v8-clone module) instead of lodash |
|
Have a try, but I just want pure javascript, especially for Windows. |
|
@popomore Native modules should work fine on windows, you can make it an optionalDependency in the package.json and fallback to lodash if it fails to install |
|
Fixed by #32 |
option
{ contents: false }causes the clone to reference-copy buffer contents.Custom properties that exist in the original file are deep-cloned to the new File.