Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not let interpret doT numbers as integer, but as string #77

Closed
bwoebi opened this issue Apr 2, 2013 · 9 comments
Closed

Do not let interpret doT numbers as integer, but as string #77

bwoebi opened this issue Apr 2, 2013 · 9 comments

Comments

@bwoebi
Copy link

bwoebi commented Apr 2, 2013

{{=it.number1}}{{=it.number2}}

evaluates to var += (it.number1)+(it.number2); which results in var += (it.number1 + it.number2);. It then concatenates the resulting number to var, which is bad.

Please evaluate it as var += String(it.number1)+String(it.number2); — this is what users really want, I think.

Quick fix: change the lines 46 - 49 from:

    var startend = {
        append: { start: "'+(", end: ")+'", endencode: "||'').toString().encodeHTML()+'" },
        split: { start: "';out+=(", end: ");out+='", endencode: "||'').toString().encodeHTML();out+='"}
    }, skip = /$^/;

to:

    var startend = {
        append: { start: "'+String(", end: ")+'", endencode: "||'').toString().encodeHTML()+'" },
        split: { start: "';out+=String(", end: ");out+='", endencode: "||'').toString().encodeHTML();out+='"}
    }, skip = /$^/;
@Pointy
Copy link

Pointy commented Oct 28, 2014

No, I assure you that that is not what users really want. You know what types your properties contain; doT doesn't. What if I had two numbers and I wanted to add them? Why would that be a less important use case?

@bwoebi
Copy link
Author

bwoebi commented Oct 28, 2014

@Pointy because that's definitely not what you expect from something which clearly should output. {{=var}} should just output what's in var. It's documented as an output operation, then it should just output too, not do any additions.

@Pointy
Copy link

Pointy commented Oct 28, 2014

@bwoebi I apologize for not expressing myself more clearly; also, I think I better understand what you're asking for now. However, I'm a little confused. If it.number is a number, and it.string is a string, then

out = (it.number)+(it.string);

will result in a string concatenation operation, and NaN cannot be generated by that. (Maybe I still don't understand what the failure scenario is.)

@bwoebi
Copy link
Author

bwoebi commented Oct 28, 2014

@Pointy Hmm, I think I've done something wrong in the original bug report. The failure is definitely present when both operands are numeric. Then they definitely add up, which my quick fix above fixes.

@Pointy
Copy link

Pointy commented Oct 28, 2014

Right, OK now that I understand, and I agree that I'd be pretty surprised by that (though I confess it's never happened to me, and I've got a lot of templates). I wonder if a simpler change that involved simply making sure that the out variable is initialized to an empty string would fix it?

@bwoebi
Copy link
Author

bwoebi commented Oct 28, 2014

@Pointy Not really. The operands on the right are evaluated first before it's concatenated to the left...

@bwoebi bwoebi closed this as completed Oct 28, 2014
@bwoebi bwoebi reopened this Oct 28, 2014
@Pointy
Copy link

Pointy commented Oct 28, 2014

@bwoebi yes I've been playing with it and I see what you mean; any time that numbers are involved things can get weird.

olado added a commit that referenced this issue Nov 24, 2014
olado added a commit that referenced this issue Nov 24, 2014
… in examples/express. And sporting new logo by @KevinKirchner
@olado
Copy link
Owner

olado commented Nov 24, 2014

Oh yes, I see this can happen if numbers are somewhere in the middle of the template.
I made changes in the dev branch for this, please take a look and try it out.
I'll merge the changes into master after a couple of days if all looks good.

olado added a commit that referenced this issue Dec 2, 2014
Version 1.0.3: addressing issues #121, #106, #77, added examples/express, new logo and some cleanup
@olado
Copy link
Owner

olado commented Dec 2, 2014

Addressed in version 1.0.3, closing

@olado olado closed this as completed Dec 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants