Spacebrew / spacebrew

A dynamic re-routable software toolkit for choreographing interactive spaces.
MIT License
222 stars 50 forks source link

"Safetify" won't work over undefined reference #56

Closed violy closed 8 years ago

violy commented 9 years ago

tested only on osX

quinkennedy commented 8 years ago

@robotconscience my instinct is to discard this pull request since it has been a while, and as I point out in #55 Node 0.10 is no longer supported by Node.js. Maybe I'm being too harsh, seeing as someone just ran into this problem this week.

robotconscience commented 8 years ago

Hm, I think it's a pretty minimal PR, though. And based on your comment at the end, I think we should merge it! Your call, though,

quinkennedy commented 8 years ago

I'm slightly concerned about including commit 539ed07 with this merge, but I don't think it will unnecessarily obscure future bugs.

robotconscience commented 8 years ago

We could do:

Handlebars.registerHelper('Safetify', function(val){
   // warn on null value
   if ( val === undefined || val = null ){
     console.log("[Spacebrew] Warning: bad value passed to 'registerHelper'");
   }
   return String(val).Safetify();
  }
);        
quinkennedy commented 8 years ago

I just tested violy/master using Node 5.6.0 and got a core dump when a client registers. won't merge. I believe 0.10 is far enough ago that we should not be adding special handling for it. changing Safetify can be handled under a separate PR.