Opened 9 years ago

Closed 8 years ago

#12888 closed enhancement (fixed)

Make debugging this.inherited(arguments) easier

Reported by: iCanDo Owned by: Eugene Lazutkin
Priority: high Milestone: 1.7
Component: Core Version: 1.6.1
Keywords: debug debugging dojo declare inherited Cc:
Blocked By: Blocking:

Description

(* For easier understanding see attached file)

Debugging methods, that invoke this.inherited * could be greatly simplified (only 2 steps (F10,F11)), if the method name is renamed to inheritedOrg (example) and a new function is added:

function inherited(args, a, f) {
	var func = this.getInherited(args, a);
	if( func ) { return func.apply(this, a || args); }
}

getInherited changed:

function getInherited(name, args){
	if(typeof name == "string"){
		return this.inheritedOrg(name, args, true);
	}
	return this.inheritedOrg(name, true);
}

For performance reasons, i use a patched version of declare.js for dev and the original for produktion.

Attachments (2)

declare.dev.js (29.6 KB) - added by iCanDo 9 years ago.
patched version of declare.js from _base
declare.patch (1.1 KB) - added by iCanDo 9 years ago.

Download all attachments as: .zip

Change History (16)

Changed 9 years ago by iCanDo

Attachment: declare.dev.js added

patched version of declare.js from _base

comment:1 Changed 9 years ago by bill

Owner: set to Eugene Lazutkin

Stepping through this.inherited() calls in the debugger also drives me crazy (since you have to step over so many lines) although Eugene was very particular about the performance of that code so he may be reluctant to add an extra function call.

BTW, we greatly prefer patches in patch file format (unified diff), and can only look at them after you file a CLA.

comment:2 Changed 9 years ago by Eugene Lazutkin

Actually I wanted to add this functionality originally --- the code is structured this way on purpose. The trick is to do it with minimal code and without affecting the performance in non-debug mode.

Changed 9 years ago by iCanDo

Attachment: declare.patch added

comment:3 in reply to:  2 Changed 9 years ago by iCanDo

Replying to bill:

BTW, we greatly prefer patches in patch file format (unified diff), and can only look at them after you file a CLA.

I added a patch file and downloaded the CLA. I will look at the CLA and send it.

Replying to elazutkin:

The trick is to do it with minimal code and without affecting the performance in non-debug mode.

Is it possible, to add a flag (debugMode=true) to the build script, so it is possible to use the original version for production and the dev version if flag debugMode is set.

comment:4 in reply to:  1 ; Changed 9 years ago by iCanDo

Replying to bill:

can only look at them after you file a CLA.

I have send CLA via facsimile.

My username is different from realname, how will i connect the CLA to my user account?

comment:5 in reply to:  4 Changed 9 years ago by Eugene Lazutkin

Replying to iCanDo:

My username is different from realname, how will i connect the CLA to my user account?

Either mention it here or email to me privately (my username at dojotoolkit.org) so I can verify and proceed.

comment:6 Changed 9 years ago by Eugene Lazutkin

I verified your CLA.

comment:7 Changed 9 years ago by Eugene Lazutkin

Resolution: fixed
Status: newclosed

(In [24998]) declare: implemented "debug" mode triggered by dojo.config.isDebug, thx iCanDo!, !strict, fixes #12888.

comment:8 Changed 9 years ago by Eugene Lazutkin

(In [25002]) declare: typo fix, !strict, refs #12888.

comment:9 in reply to:  7 ; Changed 9 years ago by iCanDo

Resolution: fixed
Status: closedreopened

Replying to elazutkin:

(In [24998]) declare: implemented "debug" mode triggered by dojo.config.isDebug, thx iCanDo!, !strict, fixes #12888.

Is it possible, to change the function signature to

function inherited__debug(args, a){ var f = this.getInherited(args, a);

so the newline after the open brace is removed.

This will reduce the steps for debugging from 3 (F10,F10,F11) to 2 (F10,F11).

comment:10 in reply to:  9 ; Changed 9 years ago by Eugene Lazutkin

Replying to iCanDo:

This will reduce the steps for debugging from 3 (F10,F10,F11) to 2 (F10,F11).

I am not sure it is worthwhile --- sometimes I need to see what function was found and what the reason was, so I do need to step in getInherited(). My understanding is that your version bypasses it.

comment:11 Changed 9 years ago by bill

FWIW, I agree with Eugene. It also doesn't match our coding standards, and it I doubt it really reduces the number of lines you step through overall, because presumably you are stepping through lots of code unrelated to the this.inherited() call.

comment:12 in reply to:  10 Changed 9 years ago by iCanDo

Replying to elazutkin:

My understanding is that your version bypasses it.

No, my version stop at getInherited(), if you press F11 instead F10 it step into getInherited().

comment:13 Changed 9 years ago by bill

Starting with [24998] the declare() unit tests gets failures in inheritedExplicitCall. (util/doh/runner.html?testModule=dojo.tests._base.declare).

comment:14 Changed 8 years ago by Eugene Lazutkin

Resolution: fixed
Status: reopenedclosed

(In [25222]) declare: fixed a bug in the debug version of inherited(), thx Bill for noticing!, !strict, fixes #12888.

Note: See TracTickets for help on using tickets.