Envjs envjs → 1.2.x

Comments and changes to this ticket

• 

  Thatcher April 14th, 2010 @ 05:29 PM

  • State changed from “new” to “open”
  • Tag set to “delete”
  • Assigned user set to “Thatcher”
  • Milestone set to “1.2.x”

  checking into this, but generally I understand this issue. I'm hoping none of this is in the html 5 parser.

• 

  nickg April 14th, 2010 @ 05:34 PM

  howdy,

  none is in html5 parser....

  $ find src -name '*.js' | xargs grep -n 'delete '
  // (edited)
  src/parser/domparser.js:43:        delete tmpNode;
  src/parser/domparser.js:49:        delete tmpNode;
  src/parser/domparser.js:51:    delete tmpdoc,
  src/parser/domparser.js:115:        delete tmpNode;
  src/parser/domparser.js:129:            delete tmpNode;
  src/parser/domparser.js:131:        delete tmpdoc;
  src/parser/domparser.js:132:        delete htmlstring;
  src/window/window.js:271:                delete scope;
  src/window/window.js:272:                delete this;
  

  Thought there were more actually...

• 

  Thatcher April 14th, 2010 @ 05:37 PM

  I think each of these could be replaced with 'foo = null;' but I need to
  double check.

• You flagged this item as spam.
  

  gleneivey April 15th, 2010 @ 11:05 AM

  • Tag changed from “delete” to “garbage collection, memory leak, delete”

  I just pushed 41401804df70f9e2de229a82ba081385377aa186 to thatcher/env-js/master. This eliminates all of the delete operation complaints from both jslint and "rhino -strict". Most of the deletes were obviously (a) intended as garbage-collection (even though that's not what 'delete' is for) and (b) completely non-functional--in fact, I'm amazed that some of these produced only warnings and not failures.

  I simply eliminated a lot of the delete operators, without even replacing them with "fu = null", as they were applied to local variables that shouldn't need any explicit aid for garbage collection. For the rest, I tried to figure out from context whether a "delete" or "fu = null" assignment was appropriate, and leave only one in place.

  All the existing tests run clean with these changes. I ended up not adding any new tests, but did do some interactive testing to make sure that code was doing what I thought it was before I tweaked it. Also, except for a couple of remove-outright's in window/window.js, it seemed like the delete's really weren't that closely related to scoping issues, so (obviously) I decided not to wait to make these changes until I was visiting those files later on.

• 

  nickg April 16th, 2010 @ 04:56 AM

  ahhh,just looked at changes

  delete foo.bar
  delete foo['bar']

  are all ok (and in one case necessary)

  it's the ones like

  delete foo
  delete this

  that are bad.

• 

  nickg April 16th, 2010 @ 05:00 AM

  ignore me.

  The changes seem fine. I'll look into urlparse and adjust as needed.

  thanks for doing this!

  --nickg