evilstreak / markdown-js

A Markdown parser for javascript
7.69k stars 863 forks source link

Various #110

Closed XhmikosR closed 10 years ago

XhmikosR commented 10 years ago

Remaining issues uglify-js reports:

WARN: Side effects in initialization of unused variable check [C:\Users\xmr\Desktop\markdown-js\lib\markdown.js:1039,10]
WARN: Side effects in initialization of unused variable old_tree [C:\Users\xmr\Desktop\markdown-js\lib\markdown.js:706,10]

I'll leave these for @ashb @evilstreak to fix.

ashb commented 10 years ago

how were you running uglify-js? Could you create a script entry in packages.json along similar lines to lint?

Not sure why it thinks line 706 has side effects:

          old_tree = this.tree,

That looks pretty side-effect tree to me.

XhmikosR commented 10 years ago

Unless you plan to add a min or something like that, target, I don't think it's useful to add it in package.json.

npm install -g uglify-js
uglifyjs markdown.js --compress --mangle -o output.js

I suppose uglifu-js complains because there is a case when old_tree might not be used. maybe? Since it's in a try...finally.

ashb commented 10 years ago

Oh uglify is a compressor. I see.

I think in this case uglify is just plain wrong about the side-effects though - it's assigned outside of the try and used in the finally - finally always executes, that's why it's called finally :) Plus a straight property read can't have side-effects.

XhmikosR commented 10 years ago

Allright, how about the rest of the changes?

ashb commented 10 years ago

Looks good - thanks