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

Thimble + Dev Tools: Add a simple "console" to log values. #1675

Closed
flukeout opened this Issue Jan 25, 2017 · 18 comments

Comments

Projects
4 participants
@flukeout
Contributor

flukeout commented Jan 25, 2017

When writing JS, using console.log() to check in on variable values is really handy. Opening dev tools while using thimble isn't a good solution because the screen gets really crowded. Is there a way to intercept console.log() calls and display them in Thimble? Any other solutions, can we add support for a "thimble.log" method?

cc @gideonthomas @humphd

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Jan 25, 2017

Member

We can do a few things:

  • Automatically inject the Firebug Lite console into the preview window. We've tried this before, and it's cool
  • We could create our own "console" in the editor pane or Thimble, and have console.log(..) get converted to postMessage that is sent from the preview out to the editor.

The question becomes: how much work do you want to do to customize what it looks like.

Member

humphd commented Jan 25, 2017

We can do a few things:

  • Automatically inject the Firebug Lite console into the preview window. We've tried this before, and it's cool
  • We could create our own "console" in the editor pane or Thimble, and have console.log(..) get converted to postMessage that is sent from the preview out to the editor.

The question becomes: how much work do you want to do to customize what it looks like.

@flukeout

This comment has been minimized.

Show comment
Hide comment
@flukeout

flukeout Jan 26, 2017

Contributor

Firebug Lite isn't any smaller than normal devtools as far as I can tell, and I'd rather not spend any time hacking it to make it more compact or limited.

I'm into exploring option number 2, and don't think it would take a ton of styling. The bigger question would be about the feature set. I think at a minimum, it could just show the last console.log value that was received. We'd just have to decide how to treat different kinds of variables and if we want to offer any special styling for arrays, objects, etc.

Some placements...

Overlay from top bar...

image

Attached to bottom of preview...

image

Using some of the filetree space...

image

Contributor

flukeout commented Jan 26, 2017

Firebug Lite isn't any smaller than normal devtools as far as I can tell, and I'd rather not spend any time hacking it to make it more compact or limited.

I'm into exploring option number 2, and don't think it would take a ton of styling. The bigger question would be about the feature set. I think at a minimum, it could just show the last console.log value that was received. We'd just have to decide how to treat different kinds of variables and if we want to offer any special styling for arrays, objects, etc.

Some placements...

Overlay from top bar...

image

Attached to bottom of preview...

image

Using some of the filetree space...

image

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Feb 18, 2017

Member

There are a few existing Brackets extensions that add console panels to the bottom of the editor, which we could use as a basis for this work:

https://github.com/sixertoy/brackets-console-plus

https://github.com/ghalex/brackets-console

Member

humphd commented Feb 18, 2017

There are a few existing Brackets extensions that add console panels to the bottom of the editor, which we could use as a basis for this work:

https://github.com/sixertoy/brackets-console-plus

https://github.com/ghalex/brackets-console

@raygervais

This comment has been minimized.

Show comment
Hide comment
@raygervais

raygervais Feb 21, 2017

Contributor

Working on this for next release. Dave, can you give me some tips.

Contributor

raygervais commented Feb 21, 2017

Working on this for next release. Dave, can you give me some tips.

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Feb 21, 2017

Member

You're going to have to do a few things.

  1. Steal console.log, console.error, ... in live preview iframe, and replace them with a function that uses postMessage to send the data back to the editor to be processed.

You're going to need to add 2 files here: src/extensions/default/bramble/lib/, and follow the pattern of MouseManager and MouseManagerRemote. Essentially, you'll have a file, ConsoleManagerRemote.js and ConsoleManager.js. The *Remote one is what will get loaded into the live preview document at runtime--this is where you'll hijack the console.log functions and friends, and replace them with calls something like this:

(function(transport) {
    "use strict";

    // This is too simplistic, but you get the idea...
    function _log(s) {
        transport.send("bramble-console", s); // see note below and fixing transport for `data`
    }

    console.log = _log;

}(window._Brackets_LiveDev_Transport));

There are various existing shims for console that you can consult on how to do this well, for example: https://github.com/kayahr/console-shim. Keep it simple initially, and extend it later with new bugs.

For sending data back on the remote transport, we'll have to modify https://github.com/mozilla/brackets/blob/master/src/extensions/default/bramble/lib/PostMessageTransportRemote.js#L38-L47 so that you can pass an extra piece of data along with the message type, and do something on the other side.

You'll also have to connect your ConsoleManagerRemote.js into the live preview by adding it here: https://github.com/mozilla/brackets/blob/master/src/extensions/default/bramble/lib/PostMessageTransport.js#L185-L202

  1. Pick one of the console UI extensions I listed, and either use it, or hack it to be what you want.

See which one you like best, and pick one.

  1. Connect the data from 1) into the extension from 2)

In https://github.com/mozilla/brackets/blob/master/src/extensions/default/bramble/lib/PostMessageTransport.js#L71-L94 you'd add a case to deal with bramble-console messages and data coming from the live preview, and you'd probably trigger an event that would get caught by your extension, so it could consume this data.

  1. Figure out how to enable/disable this feature at runtime. For now, we could just make it an optional extension we load or don't and have no UI.
Member

humphd commented Feb 21, 2017

You're going to have to do a few things.

  1. Steal console.log, console.error, ... in live preview iframe, and replace them with a function that uses postMessage to send the data back to the editor to be processed.

You're going to need to add 2 files here: src/extensions/default/bramble/lib/, and follow the pattern of MouseManager and MouseManagerRemote. Essentially, you'll have a file, ConsoleManagerRemote.js and ConsoleManager.js. The *Remote one is what will get loaded into the live preview document at runtime--this is where you'll hijack the console.log functions and friends, and replace them with calls something like this:

(function(transport) {
    "use strict";

    // This is too simplistic, but you get the idea...
    function _log(s) {
        transport.send("bramble-console", s); // see note below and fixing transport for `data`
    }

    console.log = _log;

}(window._Brackets_LiveDev_Transport));

There are various existing shims for console that you can consult on how to do this well, for example: https://github.com/kayahr/console-shim. Keep it simple initially, and extend it later with new bugs.

For sending data back on the remote transport, we'll have to modify https://github.com/mozilla/brackets/blob/master/src/extensions/default/bramble/lib/PostMessageTransportRemote.js#L38-L47 so that you can pass an extra piece of data along with the message type, and do something on the other side.

You'll also have to connect your ConsoleManagerRemote.js into the live preview by adding it here: https://github.com/mozilla/brackets/blob/master/src/extensions/default/bramble/lib/PostMessageTransport.js#L185-L202

  1. Pick one of the console UI extensions I listed, and either use it, or hack it to be what you want.

See which one you like best, and pick one.

  1. Connect the data from 1) into the extension from 2)

In https://github.com/mozilla/brackets/blob/master/src/extensions/default/bramble/lib/PostMessageTransport.js#L71-L94 you'd add a case to deal with bramble-console messages and data coming from the live preview, and you'd probably trigger an event that would get caught by your extension, so it could consume this data.

  1. Figure out how to enable/disable this feature at runtime. For now, we could just make it an optional extension we load or don't and have no UI.
@raygervais

This comment has been minimized.

Show comment
Hide comment
@raygervais

raygervais Mar 2, 2017

Contributor

A few questions David, related to this implementation.
You'll see here, I've followed your starting advice and created the ConsoleManager.js, and ConsoleManagerRemote.js files. My question, is how may I test (once I integrate the recommend console shim) to see if the iframe console messages are being picked up?

I may have a few other questions as well regarding this issue after your response.

Contributor

raygervais commented Mar 2, 2017

A few questions David, related to this implementation.
You'll see here, I've followed your starting advice and created the ConsoleManager.js, and ConsoleManagerRemote.js files. My question, is how may I test (once I integrate the recommend console shim) to see if the iframe console messages are being picked up?

I may have a few other questions as well regarding this issue after your response.

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Mar 2, 2017

Member

To test things, run brackets npm start and browse to localhost:8000/src/hosted.html and you'll get a debugging version running without Thimble. You should be able to do something like this in your index.html in the editor:

<html>
<body>
<button id="btn-log">Log It</button>
<script>
  document.querySelector("#btn-log").addEventListener("click", function() {
    console.log("This should go to your console, not the main one.");
  });
</script>
</body>
</html>
Member

humphd commented Mar 2, 2017

To test things, run brackets npm start and browse to localhost:8000/src/hosted.html and you'll get a debugging version running without Thimble. You should be able to do something like this in your index.html in the editor:

<html>
<body>
<button id="btn-log">Log It</button>
<script>
  document.querySelector("#btn-log").addEventListener("click", function() {
    console.log("This should go to your console, not the main one.");
  });
</script>
</body>
</html>
@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Mar 2, 2017

Member

Also, btw, you can do a compare branch to show diffs across commits raygervais/brackets@master...add-new-console. Having said that, just open your PR for this even if it isn't done. You can always put up "Work In Progress" (WIP) PRs for feedback.

Member

humphd commented Mar 2, 2017

Also, btw, you can do a compare branch to show diffs across commits raygervais/brackets@master...add-new-console. Having said that, just open your PR for this even if it isn't done. You can always put up "Work In Progress" (WIP) PRs for feedback.

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Mar 2, 2017

Member
+    function getRemoteScript() {
+        return "<script>\n" + ConsoleManagerRemote + "<script>\n";
+    }

Should be </script>\n; when you close it

Member

humphd commented Mar 2, 2017

+    function getRemoteScript() {
+        return "<script>\n" + ConsoleManagerRemote + "<script>\n";
+    }

Should be </script>\n; when you close it

@raygervais

This comment has been minimized.

Show comment
Hide comment
@raygervais

raygervais Mar 2, 2017

Contributor

I will keep working on it, and then do the WIP PR. I noticed your last note while working on the local version of getRemoteScript. Fixed now.

Contributor

raygervais commented Mar 2, 2017

I will keep working on it, and then do the WIP PR. I noticed your last note while working on the local version of getRemoteScript. Fixed now.

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Mar 5, 2017

Member

In unrelated work, I remembered that the Firebug Lite code already does some nice console handling, which we could learn from and use. See:

https://github.com/firebug/firebug-lite/blob/master/content/firebug1.4/consoleInjector.js

Here you can see how they implement all the various console.* methods:

https://github.com/firebug/firebug-lite/blob/master/content/firebug1.4/consoleInjector.js#L260

This might be helpful as we move forward with this.

Member

humphd commented Mar 5, 2017

In unrelated work, I remembered that the Firebug Lite code already does some nice console handling, which we could learn from and use. See:

https://github.com/firebug/firebug-lite/blob/master/content/firebug1.4/consoleInjector.js

Here you can see how they implement all the various console.* methods:

https://github.com/firebug/firebug-lite/blob/master/content/firebug1.4/consoleInjector.js#L260

This might be helpful as we move forward with this.

@gideonthomas gideonthomas added this to In Progress in Technical Debt Mar 6, 2017

@raygervais raygervais referenced this issue Mar 7, 2017

Closed

Expand JavaScript Console Handling #631

6 of 8 tasks complete
@raygervais

This comment has been minimized.

Show comment
Hide comment
@raygervais

raygervais Mar 22, 2017

Contributor

@humphd, while the PR is still being reviewed, I've begun working on the Console Interface for Bramble. I had a few questions as I ported over your second suggestion . First, I'll provide what has been done so far that you may correct or confirm the steps I've taken are in the proper direction.

  1. Added ConsoleInterfaceManager.js to brackets /src/extensions/default/bramble/lib/
  2. Added consoleTheme.css to brackets /src/extensions/default/bramble/stylesheets
  3. Added console-interface.html to brackets /src/htmlContent/console-interface.html

Number three is the item of my questions, for I understand that I will have to modify the previous ConsoleManager*.js files to interact with the new manager, but I'm asking how you'd like me to inject the new html element into Bramble. With that, should the ConsoleInterfaceManager.js not be injected similarly to the other *Remote.js files?

Essentially, I'm asking how to start with the html integration for the console.

EDIT: Including link to repository / branch with compare in case you want to see the code.

Contributor

raygervais commented Mar 22, 2017

@humphd, while the PR is still being reviewed, I've begun working on the Console Interface for Bramble. I had a few questions as I ported over your second suggestion . First, I'll provide what has been done so far that you may correct or confirm the steps I've taken are in the proper direction.

  1. Added ConsoleInterfaceManager.js to brackets /src/extensions/default/bramble/lib/
  2. Added consoleTheme.css to brackets /src/extensions/default/bramble/stylesheets
  3. Added console-interface.html to brackets /src/htmlContent/console-interface.html

Number three is the item of my questions, for I understand that I will have to modify the previous ConsoleManager*.js files to interact with the new manager, but I'm asking how you'd like me to inject the new html element into Bramble. With that, should the ConsoleInterfaceManager.js not be injected similarly to the other *Remote.js files?

Essentially, I'm asking how to start with the html integration for the console.

EDIT: Including link to repository / branch with compare in case you want to see the code.

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd Mar 27, 2017

Member

Probably this is easier to do in a face-to-face conversation. The panel should get added when you do raygervais/brackets@add-new-console...raygervais:issue-1675diff-a29088a5b670d6e4e77c46ba1f97f692R145. When I see you, maybe you can explain more to me about what you're asking.

Member

humphd commented Mar 27, 2017

Probably this is easier to do in a face-to-face conversation. The panel should get added when you do raygervais/brackets@add-new-console...raygervais:issue-1675diff-a29088a5b670d6e4e77c46ba1f97f692R145. When I see you, maybe you can explain more to me about what you're asking.

@raygervais

This comment has been minimized.

Show comment
Hide comment
@raygervais

raygervais Mar 28, 2017

Contributor

@flukeout, with the guidance of @humphd, I managed to integrate the initial console user interface into Bramble. I was recommended to ask you how you'd like the console styled, including text colours, what arguments or data should be included (I'm thinking time stamps), font-faces, and layout as well.

Dave was concerned that it spanning the entire bottom may take too much room on lower-resolution devices. Furthermore, I'm aware that I have to implement a hide() function for when the user doesn't wish to see the console, which would mean perhaps we had a button to call the show() function. Did you have any ideas where this button should be placed if we were to use it?

I also wanted to ask you about the idea of keeping the console in the editor-frame alone, so that the live preview was unobstructed. Opinion?

screenshot from 2017-03-28 15-02-17

Another idea that I presented Dave with, was reducing the console's frame to the very bottom, showing only the console tool bar and perhaps instead of an close button, an open button. This would eliminate the live preview only button, which clears and then freezes the console.

image

There is also a debate on separating the various console.* functions as they did above, so that you can filter and view the ones you wish to, or if we should let all the functions display in a single pane without the filtering or buttons.

Contributor

raygervais commented Mar 28, 2017

@flukeout, with the guidance of @humphd, I managed to integrate the initial console user interface into Bramble. I was recommended to ask you how you'd like the console styled, including text colours, what arguments or data should be included (I'm thinking time stamps), font-faces, and layout as well.

Dave was concerned that it spanning the entire bottom may take too much room on lower-resolution devices. Furthermore, I'm aware that I have to implement a hide() function for when the user doesn't wish to see the console, which would mean perhaps we had a button to call the show() function. Did you have any ideas where this button should be placed if we were to use it?

I also wanted to ask you about the idea of keeping the console in the editor-frame alone, so that the live preview was unobstructed. Opinion?

screenshot from 2017-03-28 15-02-17

Another idea that I presented Dave with, was reducing the console's frame to the very bottom, showing only the console tool bar and perhaps instead of an close button, an open button. This would eliminate the live preview only button, which clears and then freezes the console.

image

There is also a debate on separating the various console.* functions as they did above, so that you can filter and view the ones you wish to, or if we should let all the functions display in a single pane without the filtering or buttons.

@flukeout

This comment has been minimized.

Show comment
Hide comment
@flukeout

flukeout Mar 28, 2017

Contributor

Hey @raygervais, this is great! Is there a PR that I can try it out in? I'll give this some thought today and tomorrow and post some thoughts / mockups.

Contributor

flukeout commented Mar 28, 2017

Hey @raygervais, this is great! Is there a PR that I can try it out in? I'll give this some thought today and tomorrow and post some thoughts / mockups.

@raygervais

This comment has been minimized.

Show comment
Hide comment
@raygervais

raygervais Mar 29, 2017

Contributor

@flukeout, I don't have a PR up yet but I can get a WIP going so that we can work on the styling and functionality in it. How would you like to proceed?

Contributor

raygervais commented Mar 29, 2017

@flukeout, I don't have a PR up yet but I can get a WIP going so that we can work on the styling and functionality in it. How would you like to proceed?

@flukeout

This comment has been minimized.

Show comment
Hide comment
@flukeout

flukeout Mar 29, 2017

Contributor

@raygervais Yeah, if it's working locally for you I think let's get a WIP PR going, it will be easier to collaborate and make decisions if we're both able to work on it.

Contributor

flukeout commented Mar 29, 2017

@raygervais Yeah, if it's working locally for you I think let's get a WIP PR going, it will be easier to collaborate and make decisions if we're both able to work on it.

@humphd

This comment has been minimized.

Show comment
Hide comment
@humphd

humphd May 12, 2017

Member

This has been merged! Closing.

Member

humphd commented May 12, 2017

This has been merged! Closing.

@humphd humphd closed this May 12, 2017

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