On Fri, Sep 19, 2008 at 12:48 AM, Evan Gilbert <[EMAIL PROTECTED]> wrote:
> On Thu, Sep 18, 2008 at 2:25 PM, Lev Epshteyn <[EMAIL PROTECTED]> wrote: > > > On Thu, Sep 18, 2008 at 4:49 PM, <[EMAIL PROTECTED]> wrote: > > > > > > http://codereview.appspot.com/5681/diff/1/2 > > > File features/opensocial-templates/base.js (right): > > > > > > http://codereview.appspot.com/5681/diff/1/2#newcode60 > > > Line 60: log = os.log; > > > I'm not clear what "log" variable this is referring to. > > > > The JsTemplate lib currently defines a noop log() function, which it > > also calls with any debug output. This code changes the function to > > actually log. > > > I'm still unclear on the difference between window['log'] and log. > > if (window['log']) { > log = os.log; > } > > seems a little weird. Are there two functions, one at window['log'] and the > other in the local variable log? log isn't a local variable here -- it's attached to the window object. There are an awful lot of new globals in this code. That's really not going to be acceptable in the long run. > > > > > > > > > > > > > http://codereview.appspot.com/5681/diff/1/2#newcode62 > > > Line 62: window['log'] = os.log; > > > It seems a little dangerous to be overriding variables in the global > > > scope. Is window['log'] required for JsTemplates logging? And is there > a > > > longer version that might be less likely to conflict with user-defined > > > variables? > > > > > > > Unfortunately, "log()" is the function that JST currently uses. We can > > see about changing that, but that would take some time. > > > > > http://codereview.appspot.com/5681 > > > > > >
