Only show debug log in development mode #380
Conversation
There was a problem hiding this comment.
As mentioned in review comment, I'd be hesitant to remove the debug logs without some way to re-enable them. They can be useful, eg for users to send us information, and I do use them when not in development mode sometimes as well to see parse tree, exact command, etc
Co-authored-by: Pokey Rule <pokey.rule@gmail.com>
Co-authored-by: Pokey Rule <pokey.rule@gmail.com>
There was a problem hiding this comment.
Ok the last sticking points for me are:
- What happens if you want to see the initial debug output? I guess maybe the answer is that if you really want that debug output you run extension in debugger? Mixed feelings
- Worth confirming that this doesn't remove debug output from other extensions
|
|
I have just tested with another extension and we will disable debug logs for all extensions as it seems. The way around this is for cursorless to use its own logging function which we can disable. |
Hmm. Yeah I'm definitely not in love with that. That's crazy that there's so little sand boxing between extensions. But does it disable it only when you call the other extension's function from cursorless, or always for that extension? |
|
I didn't call it from cursorless and still the debug log was disabled. |
|
Now we have our own debug logger which we can disable without affecting other extensions |
| private disableDebugLog() { | ||
| this.active = false; | ||
| if (this.disposableSelection) { | ||
| this.disposableSelection.dispose(); |
There was a problem hiding this comment.
do we need to set this to null? or is it idempotent?
There was a problem hiding this comment.
You shouldn't be able to end up in disable twice without having enabled in between. I tried toggling it back and forward without any problem.
There was a problem hiding this comment.
no but the issue is that if the extension gets deactivated it will get called twice. probably not a big deal, but just being fastidious
There was a problem hiding this comment.
eg if you enable log, then disable it, then deactivate extension
| this.graph.debug.log(err.message); | ||
| this.graph.debug.log(err.stack); |
There was a problem hiding this comment.
I'm tempted to make these just console.error, because that way we can catch the stack trace the first time an error occurs. WDYT?
There was a problem hiding this comment.
I was also thinking that.
No description provided.