Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-10108] MobileWeb: Ti.API.log INFO level messages do not display as correct type #4057

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions mobileweb/titanium/Ti/API.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ define(["Ti/_/Evented", "Ti/_/lang"], function(Evented, lang) {

// the order of these DOES matter... it uses the last known function
// (i.e. if trace() does not exist, it'll use debug() for trace)
fns = ["debug", "trace", "error", "fatal", "critical", "info", "notice", "log", "warn"];

fns = ["debug", "trace", "error", "fatal", "critical", "notice", "warn", "info"];
// console.*() shim
con === void 0 && (con = global.console = {});

// make sure "log" is always at the end
["debug", "info", "warn", "error", "log"].forEach(function(c) {
fns.forEach(function(c) {
con[c] || (con[c] = ("log" in con)
? function () {
var a = Array.apply({}, arguments);
Expand All @@ -38,6 +37,12 @@ define(["Ti/_/Evented", "Ti/_/lang"], function(Evented, lang) {
})(fns[i]);
}

api.log = function () {
var a = lang.toArray(arguments);
var fn = ~afns.indexOf(('' + a[0]).toLowerCase()) && a.shift().toLowerCase();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a[0] and a.shift() return the same thing. I would do

a = ('' + a[0]).toLowerCase();

and just use that in both locations.

EDIT: on second thought, you don't need to check if afns contains the level because it's implicit in in the api[] reference. This also means you can get rid of the afns definition

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collapse the two var statements into a single var statement

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore my previous comments, I didn't realize how API.log worked. So. The way that this was intended, was that you find the name of the Ti.API.* call in fns, then crawl backwards until you find something that's in console. That way you get proper fallback to things that aren't supported (fatal and critical are error, notice is info, etc). Also, we don't have to have the extra array that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bryan-m-hughes Not sure what you mean can you give me an example to clarify.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the basic problem is that fatal, critical, and notice don't have console.* equivalents, but they shouldn't just map to info. Chris architected the fns array so that if you hit one of these, you just move to the left until you find something that does have a console.* equivalent and it's the correct equivalent. Critical and fatal move to the left until you get to error, and notice move to the left until they get to info

api[fn||'info'].apply(this, a);
};

return lang.setObject("Ti.API", Evented, api);

});