yeoman / configstore

Easily load and persist config without having to think about where and how
BSD 2-Clause "Simplified" License
868 stars 56 forks source link

Add deep nesting support for .get(), .set() and .del() #35

Closed morkro closed 8 years ago

morkro commented 8 years ago

Here is the pull request for #33 . I've added tests, updated the README a little and bumped the version.

rexxars commented 8 years ago

del doesn't work as expected here:

conf.set('foo.bar.baz', {awesome: true});
conf.set('foo.bar.bang', {awesome: false});

conf.del('foo.bar.baz');
console.log(this.conf.get('foo.bar.bang'));
// Expected: {awesome: false}
// Actual: undefined
rexxars commented 8 years ago

Also, the changes done in README stripped significant whitespace (two spaces + linebreak === hard break) - so it's probably best if you put those back :-)

Otherwise, looks good!

morkro commented 8 years ago

Thanks, I didn't realise I removed the breaks!

I couldn't reproduce your example though. It works for me. Might be that the result is undefined because you're missing this in your example: conf.set('foo.bar.baz', {awesome: true});

It should be this.conf.set('foo.bar.baz', {awesome: true});.

morkro commented 8 years ago

Okay, the breaks are somehow removed by my IDE. I added <br> to make them appear again. Hope that's fine.

rexxars commented 8 years ago

The following should reproduce the issue - paste it into the del() test:

this.conf.set('foo.bar.baz', {awesome: 'icecream'});
this.conf.set('foo.bar.zoo', {awesome: 'redpanda'});

this.conf.del('foo.bar.baz');
assert.equal(this.conf.get('foo.bar.zoo.awesome'), 'redpanda');
morkro commented 8 years ago

Updated to previous feedback.

Hope this PR is finally working for everyone. Sorry for the issues.

sindresorhus commented 8 years ago

Looks good. Thanks :)