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

Remove unnecessary console logging in LiveDevelopment code #619

Closed
humphd opened this Issue Mar 1, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@humphd
Member

humphd commented Mar 1, 2017

The LiveDev code spams the console a lot, and mostly for unnecessary things. I think it was added originally to make debugging easier. It isn't necessary now, so let's remove as much of it as makes sense:

Running git grep "console.log" src/LiveDevelopment | grep -v "node_modules" I see the following as some examples (NOTE: there are some we don't care about, since we use the MultiBrowser code paths):

src/LiveDevelopment/Documents/HTMLDocument.js:                    console.log(delta);
src/LiveDevelopment/Documents/HTMLDocument.js:            console.log("Edits applied to browser were:");
src/LiveDevelopment/Documents/HTMLDocument.js:            console.log(JSON.stringify(result.edits, null, 2));
src/LiveDevelopment/Inspector/Inspector.js:            console.log("You must connect to the WebSocket before sending messages.");
src/LiveDevelopment/LiveDevMultiBrowser.js:            console.log("Invalid launcher object: ", launcher, new Error("LiveDevMultiBrowser.setLauncher()"));
src/LiveDevelopment/MultiBrowserImpl/documents/LiveHTMLDocument.js:            console.log("Edits applied to browser were:");
src/LiveDevelopment/MultiBrowserImpl/documents/LiveHTMLDocument.js:            console.log(JSON.stringify(result.edits, null, 2));
src/LiveDevelopment/MultiBrowserImpl/protocol/remote/LiveDevProtocolRemote.js:                console.log("[Brackets LiveDev] Received message without method.");
src/LiveDevelopment/MultiBrowserImpl/protocol/remote/LiveDevProtocolRemote.js:                        console.log("[Brackets LiveDev] Error executing a handler for " + msg.method);
src/LiveDevelopment/MultiBrowserImpl/protocol/remote/LiveDevProtocolRemote.js:                        console.log(e.stack);
src/LiveDevelopment/MultiBrowserImpl/protocol/remote/LiveDevProtocolRemote.js:                console.log("[Brackets LiveDev] No subscribers for message " + msg.method);
src/LiveDevelopment/MultiBrowserImpl/protocol/remote/LiveDevProtocolRemote.js:                console.log("[Brackets LiveDev] Trying to send a response for a message with no ID");
src/LiveDevelopment/MultiBrowserImpl/protocol/remote/LiveDevProtocolRemote.js:            console.log("Runtime.evaluate");
src/LiveDevelopment/MultiBrowserImpl/protocol/remote/LiveDevProtocolRemote.js:                console.log("[Brackets LiveDev] Malformed message received: ", msgStr);

The ones that are error handling cases are probably OK. It's the ones that run by default.

@Pomax

This comment has been minimized.

Show comment
Hide comment
@Pomax

Pomax Mar 1, 2017

Is this our code, or module code? (If module, is there a newer version that might not have console logs in it anymore?)

Pomax commented Mar 1, 2017

Is this our code, or module code? (If module, is there a newer version that might not have console logs in it anymore?)

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Mar 1, 2017

Member

It's Adobe's code upstream, and for them, you don't tend to run in a browser where you care about your console being messy. We do, for obvious reasons, so it affects us more.

Member

humphd commented Mar 1, 2017

It's Adobe's code upstream, and for them, you don't tend to run in a browser where you care about your console being messy. We do, for obvious reasons, so it affects us more.

@Pomax

This comment has been minimized.

Show comment
Hide comment
@Pomax

Pomax Mar 1, 2017

I wonder if we can shim console.log for specific modules... then we could simply shim it as noop (console.log = () => {};?) without having to actually modify an upstream file.

Something like "create bundle, and anywhere in any file from src/LiveDevelopment where you see console.log, shim/rewrite it". Does require.js has some kind of alias resolver or the link that we can tap into?

Pomax commented Mar 1, 2017

I wonder if we can shim console.log for specific modules... then we could simply shim it as noop (console.log = () => {};?) without having to actually modify an upstream file.

Something like "create bundle, and anywhere in any file from src/LiveDevelopment where you see console.log, shim/rewrite it". Does require.js has some kind of alias resolver or the link that we can tap into?

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Mar 1, 2017

Member

We could, but I think there are some we'll actually care about, and separating it (esp in an on-going way) is going to be hard. I'm finding that merging with upstream isn't that horrible, even when we remove/change their code, so I'm less concerned about keeping exactly in check with what they do.

Member

humphd commented Mar 1, 2017

We could, but I think there are some we'll actually care about, and separating it (esp in an on-going way) is going to be hard. I'm finding that merging with upstream isn't that horrible, even when we remove/change their code, so I'm less concerned about keeping exactly in check with what they do.

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Mar 5, 2017

Member

Some examples:

[Brackets LiveDev] Error executing a handler for Runtime.evaluate
TypeError: Cannot set property 'selector' of undefined
    at Object.highlightRule (blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:1673:35)
    at eval (eval at evaluate (blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:144:26), <anonymous>:1:5)
    at evaluate (blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:607:26)
    at blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:540:25
    at Array.forEach (native)
    at Object.trigger (blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:537:29)
    at Object.message (blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:826:27)
    at blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:61:37

And

Runtime.evaluate
Member

humphd commented Mar 5, 2017

Some examples:

[Brackets LiveDev] Error executing a handler for Runtime.evaluate
TypeError: Cannot set property 'selector' of undefined
    at Object.highlightRule (blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:1673:35)
    at eval (eval at evaluate (blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:144:26), <anonymous>:1:5)
    at evaluate (blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:607:26)
    at blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:540:25
    at Array.forEach (native)
    at Object.trigger (blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:537:29)
    at Object.message (blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:826:27)
    at blob:http://localhost:8000/086b9a76-c00e-4549-a6b6-23677f42f128:61:37

And

Runtime.evaluate
@Abykin

This comment has been minimized.

Show comment
Hide comment
@Abykin

Abykin Mar 6, 2017

@humphd I'd like to go over the console logs, and see what can be removed/kept. Is it alright if I work on this?

Abykin commented Mar 6, 2017

@humphd I'd like to go over the console logs, and see what can be removed/kept. Is it alright if I work on this?

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Mar 6, 2017

Member
Member

humphd commented Mar 6, 2017

@Abykin

This comment has been minimized.

Show comment
Hide comment
@Abykin

Abykin Mar 6, 2017

I've gone over the logs, and picked out those who don't need to be ran, have full change log here :
#628

Abykin commented Mar 6, 2017

I've gone over the logs, and picked out those who don't need to be ran, have full change log here :
#628

@humphd humphd closed this in #628 Mar 20, 2017

humphd added a commit that referenced this issue Mar 20, 2017

commented out some of the console logs #619 (#628)
Fix #619: commented out some of the console logs

Rajat-dhyani added a commit to Rajat-dhyani/brackets that referenced this issue Apr 20, 2017

commented out some of the console logs #619 (#628)
Fix #619: commented out some of the console logs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment