search

spencermountain / Freebase.js

inference and inspection on freebase data
107 stars 14 forks source link

callback vs ps.callback #15

Closed Trott closed 9 years ago

Trott commented 9 years ago

Hey, is it me or is this line a bug?

https://github.com/spencermountain/Freebase.js/blob/06abc5e1caa8a87c101302409869699a7880b06d/src/sugar.js#L856

Shouldn't that be ps.callback rather than just callback?

Trott commented 9 years ago

Similar thing with wikipedia_links() and wikipedia_external_links(). The function code has a ps object but uses callback() rather than ps.callback(). Bug? Or do I just need to force myself to understand what fns.settle_params() is all about and all will be clear?

spencermountain commented 9 years ago

hey Rich, thanks. the ps. thing was just to denormalise the default option logic, though it didn't end up very clear. I'll check out the dig, wikipedia_links, and wikipedia_external_links issues, then cut a release you're the man

Trott commented 9 years ago

Cool. If all of those callback() instances are turned into ps.callback() then you get the node callback option for free. For any that aren't changed, I (or someone else) will need to go through them and add code to enable the node callback option.

spencermountain commented 9 years ago

i've made a bunch of changes, and not sure it all makes sense, check em out at npm install freebase@3.1.1

i think all methods support nodeCallback now, or atleast, none blow up