Clarification on the defaults argument needed
leschekfm opened this issue · 4 comments
From the documentation I would assume that the second argument for rc should always be an object.
Reviewing the code I noticed that there is also a handling for the default param being a string. I would assume that this was meant to be an option to pass a stringified JSON object as the defaults variable is handed to utils.json() then.
Looking through utils.json() the defaults var (which has to be a string at this point) is passed to utils.file(). This leads me to the conclusion that the second argument could also be a path to a .json or .ini file?
If that is right, I think it would be a good idea if this was documented.
this is a news to me actually! It should be an object, maybe it snuck in via some pull request.
Oh it seems that I wrote that line f7fe129 and called it "graceful defaults"
I guess I meant do something sensible with a string default in that position.
Though, the thing past @dominictarr wasn't taking into account is that quite likely putting a string in that position was an accident, and so this makes it passively pass but not do what you expected.
Since this was a surprise to me, and undocumented, I think we should remove that, and throw an error if the defaults object is anything but a {} object
OK that explains a lot ;)
I improved the type check and wrote some tests for variable types that could cause problems.
Please have a look at my pull request @dominictarr
I actually think that the passing a file path for default args is a good idea and should be documented.