flatiron / plates

Light-weight, logic-less, DSL-free, templates for all javascript environments!
MIT License
831 stars 69 forks source link

[fix] Call .toString on Buffer objects passed to .bind #99

Closed Southern closed 11 years ago

Southern commented 11 years ago

Fixes #90

mmalecki commented 11 years ago

That doesn't work in browsers. I think that users should do Buffer#toString explicitely instead.

Southern commented 11 years ago

@mmalecki Ah, crap. Right. Forgot this was browser based too. I'll fix it where it's compatible.

3rd-Eden commented 11 years ago

Just do html = html.toString('utf8') Also it would be nice if you added tests for it ;)

Southern commented 11 years ago

@3rd-Eden I was going to leave the error there for arrays and objects, because .toString on an object would return the [object Name] format unless it has an .inspect on it that does otherwise.

What about if (Buffer && html instanceof Buffer)?

3rd-Eden commented 11 years ago

@Southern I don't really see how html would ever be an object or an array. So I really don't see an issue here.

Southern commented 11 years ago

@3rd-Eden It shouldn't ever be an object or an array, but it should also never be a Buffer either. I suppose we could make arrays join together, but then there would have to be a delimiter setup for what to join the array items together with. Objects would be practically impossible, because of the keys. Are the keys simply ignored, or do they actually mean something? Also, objects pose the [object Object] problem if you just call .toString on it.

Buffers don't pose any of these problems, so I think it'd be easier to make it .toString on Buffers only for now.

I would say we could simply override the .toString on all objects, but Buffers pose a special case when it comes to that. (See https://github.com/joyent/node/blob/master/lib/buffer.js#L49-L85)

I guess we could do something like:

html.toString = (function (toString) {
  return function () {
    if (Buffer && html instanceof Buffer) return toString.apply(html, arguments);
    // Handle other stuff here.
  };
})(html.toString);
3rd-Eden commented 11 years ago

The use of buffers can be easily explained by the use of node's file system API's, so that is really an odd thing to expect but arrays and objects are. It seems to me that you are trying to fix a non existing issue here.

A simple html.toString() is all that it needs, it and if someone supplies an array or an object then it should just die. hard. It should expect strings or instances that can be transformed in a string.

Southern commented 11 years ago

Alright. I'll fix it that way.