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

debug.disable() does not disable? #150

Closed
swook opened this issue Oct 24, 2014 · 7 comments
Closed

debug.disable() does not disable? #150

swook opened this issue Oct 24, 2014 · 7 comments

Comments

@swook
Copy link

swook commented Oct 24, 2014

process.env.DEBUG="*";
var debug = require("debug");
var log = debug("app:log");
log("Should show");
debug.disable();
log("Shouldn't show");

The above snippet does not work as expected.

The disable function should remove namespaces instead of trying to enable nothing.
https://github.com/visionmedia/debug/blob/master/debug.js#L159

@TooTallNate
Copy link
Contributor

The disable function should remove namespaces instead of trying to enable nothing

That's not the issue.

The disable()/enable() functions will only work on future created debug instances. This is because the debug code does not maintain a list of debug instances, since it would be a memory leak technically.

I'm considering a few ways to refactor the code to remedy this, but nothing concrete so far.

@swook
Copy link
Author

swook commented Oct 25, 2014

With

var debug = require("debug");

debug.enable("app:*");
var log1 = debug("app:log1");
log1("Should show");

debug.disable();
var log2 = debug("app:log2");
log2("Shouldn't show");

debug.enable("app:*");
var log3 = debug("app:log3");
log1("Should show");
log2("Shouldn't show");
log3("Should show");

One gets

app:log1 Should show +0ms
app:log2 Shouldn't show +2ms
app:log1 Should show +1ms
app:log2 Shouldn't show +0ms
app:log3 Should show +0ms

I understand what you mean by previous debug instances not being kept track of, but the disabling still does not work since enabling "" does not disable existing entries in exports.names.

@TooTallNate
Copy link
Contributor

I see what you mean. Looks like enable() should reset the names and skips
arrays. Patch welcome, otherwise I'll get to it eventually.

On Fri, Oct 24, 2014 at 5:09 PM, Seon-Wook Park notifications@github.com
wrote:

With

var debug = require("debug");

debug.enable("app:*");
var log1 = debug("app:log1");
log1("Should show");

debug.disable();
var log2 = debug("app:log2");
log2("Shouldn't show");

debug.enable("app:*");
var log3 = debug("app:log3");
log1("Should show");
log2("Shouldn't show");
log3("Should show");

One gets

app:log1 Should show +0ms
app:log2 Shouldn't show +2ms
app:log1 Should show +1ms
app:log2 Shouldn't show +0ms
app:log3 Should show +0ms

I understand what you mean by previous debug instances not being kept
track of, but the disabling still does not work since enabling "" does
not disable existing entries in exports.names.


Reply to this email directly or view it on GitHub
#150 (comment).

@ibc
Copy link
Contributor

ibc commented Nov 16, 2014

+1

stephen added a commit to stephen/nodetunes that referenced this issue Nov 21, 2014
mgesmundo added a commit to mgesmundo/debug that referenced this issue May 13, 2015
@CumpsD
Copy link

CumpsD commented Jul 2, 2015

debug.enable(null); does a real disable, removing the localStorage in the browser version, while debug.disable() simply sets it to an empty string.

@ibc
Copy link
Contributor

ibc commented Jul 2, 2015

null == '' returns false, so @CumpsD is right and this is a bug in the code. debug.disable() should call to exports.enable(null) rather than exports.enable('').

Anyhow, calling to exports.enable(null) will just disable the debug for future created debug instances (that is what this issue is about).

@thebigredgeek
Copy link
Contributor

#370 < V3 should solve this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants