Opened 8 years ago

Closed 7 years ago

#14955 closed task (fixed)

dojox/analytics/consoleMessages plugin missing "var"

Reported by: mm Owned by: dylan
Priority: high Milestone: 1.8
Component: Dojox Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

define(["dojo/_base/lang","../_base", "dojo/_base/config", "dojo/aspect"
], function(lang, dxa, config, aspect){
	/*=====
		dxa = dojox.analytics;
		aspect = dojo.aspect;
	=====*/
	consoleMessages = lang.getObject("dojox.analytics.plugins.consoleMessages", true);

		// summary:
		//	plugin to have analyitcs return the base info dojo collects
		this.addData = lang.hitch(dxa, "addData", "consoleMessages");

		var lvls = config["consoleLogFuncs"] || ["error", "warn", "info", "rlog"];
		if(!console){
			console = {};
		}

		for(var i = 0; i < lvls.length; i++){
			if(console[lvls[i]]){
				aspect.after(console, lvls[i], lang.hitch(this, "addData", lvls[i]),true);
			}else{
				console[lvls[i]] = lang.hitch(this, "addData", lvls[i]);
			}
		}
	return consoleMessages;
});

Several questions for this source code:

Missing var in front of consoleMessages (global variable ? Purpose ? I hope not)

Using lang.getObject(, true) instead of declare() (purpose ? Avoiding dependency on base/_declare and preferring lax style instead ?)

Repeated lvls[i] lookup and lengthy code ?

Change History (13)

comment:1 Changed 8 years ago by mm

proposed rewrite (comments welcomed)

define(["dojo/_base/lang","../_base", "dojo/_base/config", "dojo/aspect"
], function(lang, dxa, config, aspect){
	/*=====
		dxa = dojox.analytics;
		aspect = dojo.aspect;
	=====*/
	var consoleMessages = lang.getObject("dojox.analytics.plugins.consoleMessages", true);

		// summary:
		//	plugin to have analyitcs return the base info dojo collects
		this.addData = lang.hitch(dxa, "addData", "consoleMessages");

		var lvls = config["consoleLogFuncs"] || ["error", "warn", "info", "rlog"];
		console || (console={});

		for(var i = 0; i < lvls.length; i++){
			var fnName=lvls[i], _addData=lang.hitch(this, "addData", fnName);
			if(console[fnName]){
				aspect.after(console, fnName, _addData , true);
			}else{
				console[fnName] = _addData;
			}
		}
	return consoleMessages;
});
Last edited 8 years ago by bill (previous) (diff)

comment:2 Changed 8 years ago by bill

Component: GeneralDojox
Description: modified (diff)
Owner: set to Adam Peller

comment:3 Changed 8 years ago by bill

Owner: changed from Adam Peller to Dustin Machi
Status: newassigned

comment:4 Changed 8 years ago by bill

mm, I'm not sure who you are? We can't take code unless you've filed a CLA, although we can add the missing "var" that you mentioned.

As for lang.getObject("dojox.analytics.plugins.consoleMessages", true), that's a typical way to create a global variable, but will disappear in 2.0 when we get rid of global variables, and will be replaced by something like

var consoleMessages = {};

comment:5 Changed 8 years ago by bill

Summary: dojox/analytics/consoleMessages plugindojox/analytics/consoleMessages plugin missing "var"

comment:6 Changed 8 years ago by mm

my CLA is filled months ago. Please check your lists.

comment:7 Changed 8 years ago by bill

I checked the lists and there is no one named "mm". What name are you listed under?

comment:8 Changed 8 years ago by mm

Martin Marko (marcus@…) Thanx

comment:9 Changed 8 years ago by mm

status ? please ?

comment:10 Changed 7 years ago by dylan

Resolution: fixed
Status: assignedclosed

In [28772]:

fixes #14955, thanks mm

comment:11 Changed 7 years ago by dylan

Milestone: tbd1.8
Priority: undecidedhigh
Resolution: fixed
Status: closedreopened

I've accepted most of this patch, but not the change for

console
(console={});

Mostly because our post-commit hook doesn't like it, and I didn't want to override it for a change that is minor.

comment:12 Changed 7 years ago by dylan

Owner: changed from Dustin Machi to dylan
Status: reopenedassigned

comment:13 Changed 7 years ago by dylan

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.