Khan / live-editor

A browser-based live coding environment.
Other
761 stars 185 forks source link

print doesn't output anything until println has been written after it #574

Open duianto opened 8 years ago

duianto commented 8 years ago

Summary

When typing: print("test"); then nothing shows up in the live editors console.

To get the print command to output anything, add a println command after the print command. for example: println("test2");

Expected Behavior

When writing a print command, then it should output to the live editors console once, without adding a new line.

Actual behavior

Nothing happens.

There's another strange behavior: When the print command doesn't output anything, then it seems to be stored somewhere, because if the print("test"); is written once and then commented and uncommented a couple of times with the hotkey: (ctrl + /). Then if println("test2"); is written below the print command, now the print command gets written to the console several times, the steps are explained below.

Steps to Reproduce

  1. type: print("test"); nothing happens
  2. type: println("test2");

The live editors console now opens and shows the following:

testtesttest
test2

For some reason the print command is written 3 times. If the println command is commented and uncommented once, then the print command is written twice:

testtest
test2

To see that the print commands output is stored somewhere until the println command is called.

Comment out the second line:

print("test");
// println("test2");

Now comment and uncomment the first line 5 times (hotkey: ctrl + /):

// print("test");
// println("test2");

Finally if the first line is commented, then uncomment it, now uncomment the second line, and the console shows:

testtesttesttesttesttesttest
test2

Environment

OS: Windows 7 Home Premium 64 bit Browser: Google Chrome Version 49.0.2623.112 m

bytorbrynden commented 8 years ago

I did what you described (minus the commenting and uncommenting), outside of the live-editor (here), and got the same result, which leads me to believe that this isn't a problem with the live-editor, but rather a problem with ProcessingJS.

Perhaps you could look to see if there's already an issue similar to this one, on the official ProcessingJS repository, and if not, open one?

When I get a chance, I'll take a look at ProcessingJS' source, and see if this is indeed a problem, and if-so, will try and take a crack at fixing it.

duianto commented 8 years ago

I might have found a solution, it's posted on the processing-js issue page.

But i stumbled over this comment from Apr 1, 2016: https://github.com/Khan/live-editor/issues/570#issuecomment-204218076 "We https://github.com/khan/processing-js which has diverged from https://github.com/processing-js/processing-js."

Then it seems like any changes made to processing-js/processing-js won't transfer to the khan/live-editor?

Here's what the khan/processing-js, print function looks like: https://github.com/Khan/processing-js/blob/master/processing.js#L9397

    p.print = function(message) {
      logBuffer.push(message);
    };

it's only pushing the print message to the logBuffer array.

Here's what the println function looks like: https://github.com/Khan/processing-js/blob/master/processing.js#L9377

    p.println = function(message) {
      var bufferLen = logBuffer.length;
      if (bufferLen) {
        Processing.logger.log(logBuffer.join(""));
        logBuffer.length = 0; // clear log buffer
      }

      if (arguments.length === 0 && bufferLen === 0) {
        Processing.logger.log("");
      } else if (arguments.length !== 0) {
        Processing.logger.log(message);
      }
    };

Maybe it's this line that opens the console: Processing.logger.log(""); then it probably should be added to the print function.

New line issue

I also noticed that on the processingjs helper page: http://processingjs.org/tools/processing-helper.html

when running these lines:

print("test");
println("test2");

then the output is printed on a single line, testtest2

but in the khan academy code editor: https://www.khanacademy.org/computer-programming/new/pjs

the same commands:

print("test");
println("test2");

are printed on two lines:

test
test2

the khan academy code editor should probably also print on the same line, since the print command is meant to print without a new line.

The println command seems to add the extra new line, because if print("test"); is called twice before a println command:

print("test");
print("test");
println("test2");

then the two print commands are printed on the same line:

testtest
test2

However if two println commands are printed after each other:

println("test2");
println("test2");

then there's no extra new line between them, they are both printed on their own line as expected:

test2
test2
bytorbrynden commented 8 years ago

Yes, Khan Academy has their own fork of ProcessingJS. I guess I never realized it was so far behind the official version. So, I guess any changes that take place, will have to take place in Khan/processing-js.

I definitely agree with you about the new-line issue. I saw that same result when I ran tests.

So, I guess the question we should be asking now, is "should print create a new line in the PJS console, if one doesn't exist" (which, I'm hoping, would actually open the console)?

duianto commented 8 years ago

In google chrome i right clicked on the console in the live editor: http://khan.github.io/live-editor/demos/simple/

print("test");
println("test2");

console output:

test
test2

and chose "Inspect", there i noticed that the "test" and "test2" lines are in their own div. I searched for where those divs are created.

The println function: https://github.com/Khan/processing-js/blob/master/processing.js#L9377 calls the command: processing.logger.log.

processing.logger is assigned on this line: https://github.com/Khan/processing-js/blob/master/processing.js#L19830 Processing.logger = tinylogLite;

and this tinylogLite function: https://github.com/Khan/processing-js/blob/master/processing.js#L19794

tinylogLite[log] = function(message) {
  if (messages === logLimit) {
    output.removeChild(output.firstChild);
  } else {
    messages++;
  }

  var entry = append(output, createElement($div)),
    entryText = append(entry, createElement($div));

  entry[$title] = (new Date()).toLocaleTimeString();

  setStyles(
  entry, entryStyles, entryText, entryTextStyles);

  append(entryText, createTextNode(message));
  output.scrollTop = output.scrollHeight;
};

seems to create a new div when a println command is called, that's probably why println ends up on the next line instead of on the same line as the output from the print command.

A possible solution

Maybe a "current console output" div could be created, and the output from the next print or println command gets appended to that div.

When a println command is called, then it ends by creating a new empty div, and that becomes the "current console output" div.

Broken url

the url: https://purl.eligrey.com/tinylog/lite on this line: https://github.com/Khan/processing-js/blob/master/processing.js#L19563

results in: "404 Not Found"

maybe the url should be changed to: https://github.com/eligrey/tinylog

Should the broken url be a separate issue?

hmm..., there doesn't seem to be a "Issues" link between "Code" and "Pull Requests" at the top of the khan/processing-js page: https://github.com/khan/processing-js

like this page has: https://github.com/Khan/live-editor

Maybe issues for khan/processing-js should be submitted here on khan/live-editor?

bytorbrynden commented 8 years ago

Yes, Khan/processing-js related issues, if I'm remebering correctly, are typically reported on this repository (see Khan/live-editor/labels/pjs).

I'll mess around with the logging in the PJS, when I get some free time.

I'm not entirely sure what to say about the broken link issue seeing as the eligrey/tinylog repository is also linking to "https://purl.eligrey.com/tinylog/lite"...

kevinbarabash commented 8 years ago

I think the reason why we don't allow issues on https://github.com/khan/processing-js is that we would no doubt get lots of issues lots of general processing-js issues that we may not want to deal with. This issue though is probably worth digging into.

For the logging it would be nice if it just append text to the current line for calls to print. I had a quick look at the code and it'd probably be easier to replace tinylog with something else that more closely matches the print/println semantics that we want.

codeHusky commented 8 years ago

I'm thinking that enabling issues on the khan processing-js repo would be a good idea. It would just kind of urge the team to keep that repo more up to date then it is. Falling being in processing versions isn't really a good thing.

eligrey commented 6 years ago

@Gigabyte-Giant @duianto I fixed the dead link.