Open rickhallett opened 6 years ago
Thanks! This is great.
We’ll want to make sure we make these changes in the source log.coffee
file instead of log.js
.
I’d also like to improve on the method name restorePreviousLogRef
, although I think that’s a good start. Here are a few candidates:
log.export()
log.moveToNamespace()
log.exportToNamespace()
log.restore()
log.assignTo()
log.useCustom()
log.exportAndRestore()
log.useCustomFunction()
log.restorePreviousLogRef()
Of all of these, I think I’m most a fan of log.export
, since it mirrors the behavior you get when you use the library with AMD or another supported dependency management system (and "exports" is the terminology there).
All of this being said, as I’m reminded that we do support RequireJS and AMD (which allow you to bring in log
into your own namespace) it makes me wonder if this is even needed. I’m still interested in pursuing this PR though. I’d be interested to see what it looks like before deciding.
Hi Adam, thanks for the feedback. Do you mean making the changes solely in log.coffee, or making them in both?
I like log.export too, but export is often a reserved keyword, according to my linter. How about exportAndRestore? I've made a commit with these changes to the coffee.log file, but relied heavily on a converter available at http://js2.coffee/. I have removed what seems to be more obviously superfluous and synchronised comments with your original work. As for a more detailed evaluation of the the code, I would have to defer to your expertise.
You’ll want to make changes to log.coffee
and then run grunt
to compiled them
PR based on issue #46
Changes suggested:
pass in the window object by parameter into the IIFE directly rather than use
call
. This patterns mimics some of the major libraries in javascript and is functionally equivalent.add
restorePreviousLogRef
function that restores thewindow.log
reference to whatever it was prior to loading the log library. This feature is exported via the exportif
conditional from line 195 of the code base. Example use case below: