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

Jump to bottom

Optimize writer #2402

Merged
merged 2 commits into from Mar 31, 2022
Merged

Optimize writer #2402

merged 2 commits into from Mar 31, 2022

Conversation

hmdne
Copy link
Member

@hmdne hmdne commented Feb 27, 2022

No description provided.

@hmdne hmdne added this to the v1.5 milestone Mar 23, 2022
@hmdne hmdne marked this pull request as ready for review March 23, 2022 10:47
@hmdne hmdne changed the title Optimize writer [wip] Optimize writer Mar 23, 2022
@hmdne
Copy link
Member Author

hmdne commented Mar 23, 2022

Also tests will need to be ported.

@elia
Copy link
Member

elia commented Mar 25, 2022

Preemptive approval, still good to add more testing before merging

@hmdne @elia
Replace the explicit_writer_return rewriter with call node logic.
e448dbc 
This allows us to optimize the AsciiDoctor code a lot, but also
brings us closer to compatibility with MRI.
@hmdne @elia
CSendNode is now handled by CallNode
6ef5abe 
Conditional send in Ruby can be used together with writers to
return a correct value.
elia
elia approved these changes Mar 30, 2022
View reviewed changes
Copy link
Member

@elia elia left a comment •
edited

Choose a reason for hiding this comment

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

Awesome! (I added a commit for simplifying the writer method name check)

@elia elia merged commit 75b01e7 into master Mar 31, 2022
@elia elia deleted the hmdne/optimize-writer branch March 31, 2022 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elia elia elia approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@hmdne @elia