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.

Sign up for GitHub

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

Jump to bottom

Convert arguments to V8 directly in EventEmitter::Emit #1009

Merged
merged 8 commits into from
Jan 16, 2015
Merged

Convert arguments to V8 directly in EventEmitter::Emit #1009

merged 8 commits into from
Jan 16, 2015

Conversation

zcbenz
Copy link
Contributor

@zcbenz zcbenz commented Jan 15, 2015

This makes use of C++11 variadic templates to get rid of the conversion between base::ListValue when sending arguments from C++ to JavaScript.

It also makes the C++ code as simple as JavaScript:

base::ListValue args;
args.AppendInteger(level);
args.AppendString(message);
args.AppendInteger(line_no);
args.AppendString(source_id);
Emit("console-message", args);

now becomes

Emit("console-message", level, message, line_no, source_id);

@bwin
Copy link
Contributor

bwin commented Jan 15, 2015

Nice!

@zcbenz zcbenz force-pushed the better-emit branch 2 times, most recently from 2e61cf9 to 397f65f Compare January 15, 2015 23:09
@zcbenz
Copy link
Contributor Author

zcbenz commented Jan 15, 2015

Some how the -stdlib=libc++ gets lost when building Inspector in travis CI.

@zcbenz zcbenz force-pushed the better-emit branch 3 times, most recently from 31e285c to b2bec7a Compare January 16, 2015 01:03
zcbenz added a commit that referenced this pull request Jan 16, 2015
@zcbenz
Merge pull request #1009 from atom/better-emit
0ac2443 
Convert arguments to V8 directly in EventEmitter::Emit
@zcbenz zcbenz merged commit 0ac2443 into master Jan 16, 2015
@zcbenz zcbenz deleted the better-emit branch January 16, 2015 01:40
This was referenced Sep 2, 2020
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zcbenz @bwin