archive.bulk() messes up filenames when data is specified #67

Closed
wants to merge 2 commits into
from

2 participants

@petrosagg

If you create an archive (with any of the default modules) of a folder that has multiple files using archive.bulk() and also set the data parameter to be an object all the files in the archive end up with the same name.

archive.bulk([
  { expand: true, cwd: 'mydir', src: ['**'], dest: 'newdir', data: {} }
]);
@petrosagg petrosagg changed the title from Fix issue #66 to archive.bulk() messes up filenames when data is specified Mar 26, 2014
@ctalkington
Archiver member

what data are you trying to set?

@ctalkington
Archiver member

is there a reason to not use _.merge here? it seems like the deep version of _.extend.

@petrosagg

I found the bug trying to to set data: {store: true} and in the beginning I thought that store was causing the issue. But even if you set an empty object it will cause it to be reused for all files.

The reason that I used _.cloneDeep is that both _.merge and _.extend make sense when the destination object is non-emtpy. _.cloneDeep is a special case of merging to an empty object that seems clearer to me as it does what it says.

@ctalkington
Archiver member

hum, ok let me peek at the code tonight and see how i want treat this.

@ctalkington
Archiver member

this is very odd, as name is actually set after this process.

@ctalkington
Archiver member

it seems that in the onend of json module, the var just suddenly is changed to level2. your patch must work because its creating an clone and not reference or something.

@ctalkington
Archiver member

thanks for bringing this to my attention. i patched it in a slightly different way. i believe its the foreach in foreach that was causing the vars by reference to get spastic.

@ctalkington
Archiver member

pushed out 0.7.1, let me know how you go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment