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

improve log output of Callable's #1213

Merged
merged 2 commits into from
May 3, 2022
Merged

Conversation

rbri
Copy link
Collaborator

@rbri rbri commented May 1, 2022

NativeJSON.stringify outputs Callable's as null, convert to string to make the log output less confusing

@p-bakker
Copy link
Collaborator

p-bakker commented May 1, 2022

You sure it should automatically stringify the function including the body?

Not at my laptop, but I think Chrome just logs [Function] (for JavaScript functions, and [native function] for non-js functions. And if you want to log the entire function incl. the body, you have to call {{function}}.toString() yourself.

Not sure how your PR handles non-JS functions

Logging the entire function source might even be a bit of a security issue, depending on the context.

@rbri
Copy link
Collaborator Author

rbri commented May 1, 2022

At least better than logging null :-D
But a good point, will have a look at your suggested improvements.

StringBuilder sb = new StringBuilder();
sb.append("function ")
.append(((BaseFunction) value).getFunctionName())
.append("() {}");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wondering whether this should not be "() {...}" to make it clearer that the body content was stripped

@p-bakker
Copy link
Collaborator

p-bakker commented May 2, 2022

LGTM

@rbri
Copy link
Collaborator Author

rbri commented May 2, 2022

@p-bakker many thanks for all the reviews

@gbrail
Copy link
Collaborator

gbrail commented May 2, 2022

This looks good to me. I'll work on merging it soon.

@gbrail gbrail merged commit 7bd2eb8 into mozilla:master May 3, 2022
@rbri rbri deleted the console-log-callables branch May 29, 2023 12:43
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.

None yet

3 participants