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

Already on GitHub? Sign in to your account

Enabling / disabling logging to avoid spam from console #6

Closed
frantzmiccoli opened this Issue Dec 7, 2012 · 13 comments

Comments

Projects
None yet
2 participants
Collaborator

frantzmiccoli commented Dec 7, 2012

Context : I'm writing on a project where there're loops that regularly output something to the history for debugging.

Usecase : Sometime, I've performed some operation on my data and I would like to have the opportunity to analyze log, but the constant log output just make it impossible, the view kept scrolling down and I end up lost in the .

Proposed solution : add a button enabling the freeze the history

I'm working on this.

@ketamynx do you mind if I do some pieces of refacto and documentation ? (mostly documentation)

Owner

Silviu-Marian commented Dec 7, 2012

This feature is partially implemented.

In debugger.js, dbg.queued is holding all messages until it gets any client connections to spit messages to. It's just a matter of disconnecting the client temporarily.

Right now there's no "temporary disconnect" option, but you can blend it along with the first button (the bottom left corner, the one that hides the console); if someone clicks it, you just have to tell the client not to connect unless that button is on.

Owner

Silviu-Marian commented Dec 7, 2012

Actually, do it however you want. I was pointing at that lower left corner button because it does something pointless now and with your change it might just do something useful. :P

Collaborator

frantzmiccoli commented Dec 7, 2012

I find it useful, I'm usually outputing data with console.log on my server code and not using the console that much.

What about refacto and documentation ?

Owner

Silviu-Marian commented Dec 7, 2012

I just made you a collaborator. I think you can run commits directly now, just make sure to do all the testing first please.

Collaborator

frantzmiccoli commented Dec 7, 2012

Thanks.

I think it's best if you still review my commit, just as a matter of process four eyes are better than two. And since you're the original developer of the project yours might see errors than mine can't.

I'm thinking of another implementation, purely on the client side. You'll tell me what you think of it.

Owner

Silviu-Marian commented Dec 7, 2012

All right then. I'd be really happy with a client side reimpl.; just keep all existing features please. I don't know if it's still in the docs, but I wrote this originally with server-sent events in mind. And because they didn't work, it's working with suspended connections now.

Just a small thing please, don't get it run on external dependencies. I know websockets too, and I know they're in RFC for a while now but having the project lightweight, stand-alone and simple is just easier for everyone.

Collaborator

frantzmiccoli commented Dec 7, 2012

ketamynx#8

No external dependencies here, I've tried to stay close to what you've already done. I might have expanded some functions for readability.

No doc added yet.

Owner

Silviu-Marian commented Dec 7, 2012

Question, why transmit the whole message stack to the client anyway?
Have a look in scripts/sse.js and scripts/tools.js, all that had to be done was to put some conditions in there.
Don't connect to the server unless the input textarea is visible.

Collaborator

frantzmiccoli commented Dec 7, 2012

Well, I've chosen to do it purely client side (simplified the knowledge I had to have over the whole code and the implementation). Since node-codein is atool for debugging I didn't think the overhead was a problem. This also enables to notify the user of the hidden messages (although that could be done without sending the whole message).

Owner

Silviu-Marian commented Dec 7, 2012

Actually I think it's better this way; on two connected clients, the server would still bleed messages to the second one instead of keeping them. Merging now, but I'll test everything later.

Silviu-Marian added a commit that referenced this issue Dec 7, 2012

Merge pull request #8 from frantzmiccoli/issue-6
Issue #6 : enable/disable new messages button
Owner

Silviu-Marian commented Dec 7, 2012

Nope, not good; problems in random order:

  • the animated icon is somewhat nice, but becoming very distracting after a while
  • button title is irrelevant
  • message skip count is way off, it probably counts entered commands too
  • upon halting messages, re-enabling doesn't put them back automatically, I was thinking more in the lines of "freeze/unfreeze"
  • after re-enabling and new message received, it will simply spam like there's no tomorrow;
  • it doesn't even record entered commands while paused, this should be the case
  • the whole change seems impractical

I temporarily reverted the merge. I think there are better ways to approach this.
If you really must run loops with console.log(), just put a conditional if(global.pauselogging) continue; in the loop and in the console write global.pauseloop=1; and hit enter.
Because this feature... not even Chrome's inspector has it and isn't much of an innovation either

Collaborator

frantzmiccoli commented Dec 7, 2012

You made an interesting point with that global.pauselogging idea, that's a nice point I should consider.

You suggest we drop the feature or try to fix to bugs/bad behaviors you noticed ?

Owner

Silviu-Marian commented Dec 7, 2012

I think we should forget about it. My code is upside down and the problem's so damn easy that it doesn't deserve a feature on its own.

This console should replicate >90% of the features that Chrome's Inspector has. I wrote it from a lack of viable alternatives, but that's all the reason why it doesn't have any additional bells and whistles. It's just "Inspector for node" if you will.

Thanks again.

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