Skip to content
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

Console should always flush after writing a newline #1726

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

andreabergia
Copy link
Contributor

@andreabergia andreabergia commented Nov 21, 2024

Not really sure this deserves a big description. 🙂
We just want to ensure that any console.log actually ends up being printed immediately and not buffered.

@gbrail
Copy link
Collaborator

gbrail commented Nov 25, 2024

Actually...

I'm not convinced that this is a good idea. "ShellConsole" is a thin abstraction that wraps various types of output streams (PrintWriters, which wrap PrintStreams, in this case), that already has the same abstraction for printing and flushing that Java streams have had forever.

Adding "flush" calls is something that seems trivial and will probably help in some cases, but in others it could have a huge performance impact, and I'd rather not break those folks.

Depending on the use case, MAYBE console.log itself (or perhaps the ShellConsolePrinter) class should flush, but I'm not even sure of that.

Can you talk about the use case that's driving this?

@andreabergia
Copy link
Contributor Author

Ok, what you say makes sense.
Our use case was simple - we were debugging rhino (in the java debugger), and we couldn't see the stuff that was printed by our JS code using console.log.

I'm going to change this so that console.log actually flushes.

@gbrail
Copy link
Collaborator

gbrail commented Nov 27, 2024

OK -- that makes sense then. Thanks!

@gbrail gbrail merged commit 19092d1 into mozilla:master Nov 27, 2024
3 checks passed
@andreabergia andreabergia deleted the console-always-flush branch November 27, 2024 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants