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 macro methods for MacroIf and MacroFor nodes #13902

Merged

Conversation

sbsoftware
Copy link
Contributor

This fixes a small part of #3274 (cc @HertzDevil). I implemented these methods because I need them for a DSL macro that alters every node of a given block. It should leave macro ifs and fors as they are while still altering their bodies, so I needed a way to reconstruct those nodes from their parts.

This is my very first PR for Crystal, please let me know if anything is not as it's supposed to be!

@sbsoftware sbsoftware changed the title Add macro methods for MacroIf and MacroFor methods Add macro methods for MacroIf and MacroFor nodes Oct 24, 2023
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

I just have some tiny suggestions to fix doc typography.

src/compiler/crystal/macros.cr Outdated Show resolved Hide resolved
src/compiler/crystal/macros.cr Outdated Show resolved Hide resolved
src/compiler/crystal/macros.cr Outdated Show resolved Hide resolved
src/compiler/crystal/macros.cr Outdated Show resolved Hide resolved
src/compiler/crystal/macros.cr Outdated Show resolved Hide resolved
src/compiler/crystal/macros.cr Outdated Show resolved Hide resolved
@straight-shoota straight-shoota added this to the 1.11.0 milestone Oct 27, 2023
@straight-shoota straight-shoota merged commit 856cb21 into crystal-lang:master Oct 28, 2023
53 of 55 checks passed
@sbsoftware sbsoftware deleted the add-macro-astnode-methods branch October 28, 2023 17:04
Blacksmoke16 pushed a commit to Blacksmoke16/crystal that referenced this pull request Dec 11, 2023
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.

5 participants