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

Use debug from npm instead of custom debug solutions #84

Merged
merged 9 commits into from
May 24, 2017
Merged

Conversation

j-f1
Copy link
Contributor

@j-f1 j-f1 commented May 14, 2017

This uses npm’s debug module to manage logging.

Copy link
Member

@ArtOfCode- ArtOfCode- left a comment

Choose a reason for hiding this comment

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

I'd like to keep the original data that was getting logged, for FDSC at least. L114/118, for example.

@j-f1
Copy link
Contributor Author

j-f1 commented May 14, 2017

@ArtOfCode- Why?

@ArtOfCode-
Copy link
Member

@j-f1 Mostly because in all likelihood I did it like that for a reason

@ArtOfCode-
Copy link
Member

^ FDSC approved, other script authors should look at their own stuff

Copy link
Member

@ferrybig ferrybig left a comment

Choose a reason for hiding this comment

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

Regarding Spamtracker:

I think this will improve spam tracker, the only confusing factor I see is the fact that debug is now placed as the rootlogger, so it is harder to actually see what messages are the debug category. Also, because the fact that this PR hides the "error" messages by default, it makes debugging harder, in the case something goes wrong with the script.

Copy link
Member

@Cerbrus Cerbrus left a comment

Choose a reason for hiding this comment

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

Looks ok 👍

Copy link
Member

@ferrybig ferrybig left a comment

Choose a reason for hiding this comment

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

Regarding Spamtracker:

The script is now broken:

ERROR: Execution of script 'Spamtracker' failed! createDebug is not a function

Copy link
Member

@Glorfindel83 Glorfindel83 left a comment

Choose a reason for hiding this comment

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

I've never used the debug module before, but it makes sense to use a single one throughout all our userscripts.

/* eslint-disable max-nested-callbacks */

(function () {
"use strict";
const createDebug = typeof unsafeWindow === "undefined" ? window.debug : unsafeWindow.debug || window.debug;
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this be functionally equal, but cleaner?

const createDebug = (unsafeWindow || window).debug;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Cerbrus If unsafeWindow is undefined, that’ll throw a ReferenceError.

Copy link
Member

Choose a reason for hiding this comment

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

Aw shoot, you're right @j-f1. That's a shame.

@j-f1
Copy link
Contributor Author

j-f1 commented May 15, 2017

@ferrybig Does Spamtracker work again?

@ferrybig
Copy link
Member

@j-f1 No, Still the same error.

@ jf After a look through the code, it seems UnsafeWindow is defined as window using the IIFE, for some reason your script is probably included in the global window scope, we can add another argument for this "UserScriptManagerWindow" argument

https://chat.stackexchange.com/transcript/message/37405109#37405109

@j-f1
Copy link
Contributor Author

j-f1 commented May 15, 2017

@ferrybig Are you using Greasemonkey?

@ferrybig
Copy link
Member

@j-f1 tamper monkey on Google chrome.

Could it be the case that it works for you because another userscript is importing the debug library in the window variable, as haven't installed the other userscripts

"use strict";
const createDebug = typeof unsafeWindow === "undefined" ? window.debug : unsafeWindow.debug || window.debug;
const createDebug = typeof orginalWindow === "undefined" ? window.debug : orginalWindow.debug || window.debug;
Copy link
Member

Choose a reason for hiding this comment

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

s/orginal/original/g

@j-f1
Copy link
Contributor Author

j-f1 commented May 23, 2017

I’m planning to merge this today. Any objections?

@j-f1 j-f1 merged commit e32e59b into master May 24, 2017
@j-f1 j-f1 deleted the use-debug branch May 24, 2017 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants