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

Add console logger for web to log warnings to console #99256

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

girdenis-p
Copy link
Contributor

@girdenis-p girdenis-p commented Nov 14, 2024

Resolves #98952. Adds a platform specific logger to the web which allows to print through console.warn in the browser and which links to the JS EngineConfig API.

This also seemed like an appropriate time to change the docstring to reflect the implementation more accurately. See comment on the docstring for a further explanation.

This PR is in draft because this needs discussion:

  • Do we need the ability to log warnings as warnings in the console instead of errors?
  • Consecutive printraw functions now log on seperate lines instead of the same line. This behaviour is similar to calling consecutive console.log functions. What is desired?
  • Prints containing newlines used to be seperated but now they are grouped into one log. What is desired?

*
* By default, ``console.log()`` is used.
*
* @callback EngineConfig.onPrint
* @param {...*} [var_args] A variadic number of arguments to be printed.
* @param {string} text The text to be printed.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous docstring said that this callback takes a variadic number of arguments to be printed. However, this callback has only ever took one argument to it because Module.print only takes one argument.

Variadic arguments are handled differently in JS than in C++ so any implementation of this function utilizing the variadic arguments through the spread operator ... would still work as intended even after this change.

@Calinou
Copy link
Member

Calinou commented Nov 25, 2024

  • Do we need the ability to log warnings as warnings in the console instead of errors?

I'd say warnings being logged as errors is a bug, so yes. We need to categorize them correctly so browser's log filtering options function as intended.

  • Consecutive printraw functions now log on seperate lines instead of the same line. This behaviour is similar to calling consecutive console.log functions. What is desired?

I would prefer printraw() to work like it does on native platforms, i.e. print a message without a newline at the end so you can append to it later on (until you do another print(), that is). However, this isn't critical if we can't get it to work as it's seldom used.

  • Prints containing newlines used to be seperated but now they are grouped into one log. What is desired?

I'd say this is desired, so that you can collapse the entire message using the browser's DevTools functions.

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

Successfully merging this pull request may close these issues.

Warnings are printed as errors on the web platform
3 participants