Opened 13 years ago

Closed 12 years ago

#2251 closed defect (wontfix)

do not run require commands if it is commented out by // when debugAtAllCosts=true

Reported by: liucougar Owned by: liucougar
Priority: high Milestone: 0.9
Component: General Version: 0.4.1
Keywords: Cc:
Blocked By: Blocking:

Description

the patch prevents executing of source code like this:

//	dojo.require("dojo.widget.Editor2Plugin.ContextMenu");

(code commented out by /* */ will still be executed though, but at least this patch has fixed part of the problem before removeComments is fixed to strip comments reliably)

Attachments (2)

2251.patch (1.3 KB) - added by liucougar 13 years ago.
2251.2.patch (793 bytes) - added by liucougar 13 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 13 years ago by liucougar

updated the regex according to peller's advice; a bit more comments

Changed 13 years ago by liucougar

Attachment: 2251.patch added

Changed 13 years ago by liucougar

Attachment: 2251.2.patch added

comment:2 Changed 13 years ago by liucougar

2251.2.patch does the trick another way:

it will only match dojo.require beginning with spaces or nothing

which do you think is better?

comment:3 Changed 13 years ago by James Burke

I think 2251.2.patch will not work if the user has more than one dojo.require() call on a line. While I don't think that is too big of an issue right now, I hope we can later allow for "minifying" all Dojo modules as part of a build process, and in that minify case (where excessive white space/line and line returns are removed), then it might be an issue.

I favor changing removeComments to only do single-line comment removal, but keep all the old code and comment so we know it is not ready for general use. Then, call removeComments before calling the getRequiresAndProvides and getDelayRequiresAndProvides functions (the functions that look for the dojo.require/dojo.requireIf dependencies). Even though removeComments won't be perfect (it might have issues with in strings and regexps), it should be fine for using it when looking for dojo.require/requireIf dependencies.

comment:4 Changed 12 years ago by Adam Peller

Resolution: wontfix
Status: newclosed

debugAtAllCosts is gone, so this is no longer an issue

Note: See TracTickets for help on using tickets.