Yes, this is currently a problem. We have tried to put all of our
identifiers into a namespaced object, but JSTemplate doesn't yet work
that way.

In the initial implementation the code is all wrapped up in an
anonymous function. We are looking to make that library more friendly
to use "out of the box".

On Thu, Sep 18, 2008 at 6:52 PM, Kevin Brown <[EMAIL PROTECTED]> wrote:
> 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
>> > >
>> >
>
>

Reply via email to