Skip to content

fix: set input_lexbuf when reading binary AST too#483

Merged
NathanReb merged 2 commits intoocaml-ppx:trunk-supportfrom
anmonteiro:anmonteiro/lexbuf-for-binary-too
Mar 26, 2024
Merged

fix: set input_lexbuf when reading binary AST too#483
NathanReb merged 2 commits intoocaml-ppx:trunk-supportfrom
anmonteiro:anmonteiro/lexbuf-for-binary-too

Conversation

@anmonteiro
Copy link
Copy Markdown
Contributor

@anmonteiro anmonteiro commented Mar 22, 2024

@anmonteiro anmonteiro mentioned this pull request Mar 22, 2024
Closed
@anmonteiro anmonteiro force-pushed the anmonteiro/lexbuf-for-binary-too branch from 12cf329 to db788a9 Compare March 22, 2024 05:08
@NathanReb
Copy link
Copy Markdown
Collaborator

NathanReb commented Mar 22, 2024

@anmonteiro I'm not super familiar with melange but I'm guessing this is necessary for reason's syntax support right?

With reason you're using the driver on the marshalled AST resulting from parsing the reason syntax, whereas with regular OCaml syntax, you're using the driver directly on the source files. Are my assumptions correct?

@anmonteiro
Copy link
Copy Markdown
Contributor Author

anmonteiro commented Mar 22, 2024

I don’t think so. There’s no dialect present in my repro https://github.com/anmonteiro/no-source-quotation-repro

I think this will rather happen on all staged_pps (the instructions in my repro pass the -ppx flag)

@NathanReb
Copy link
Copy Markdown
Collaborator

NathanReb commented Mar 26, 2024

Ok thanks for the details!

I'll add a test for this and merge!

@NathanReb
Add test case for source quotation with binary AST input
6dcf298 
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
@NathanReb NathanReb force-pushed the anmonteiro/lexbuf-for-binary-too branch from db788a9 to 0f3350c Compare March 26, 2024 14:20
@anmonteiro @NathanReb
fix: set input_lexbuf when reading binary AST too
b6e4c93 
Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
@NathanReb NathanReb force-pushed the anmonteiro/lexbuf-for-binary-too branch from 0f3350c to b6e4c93 Compare March 26, 2024 14:26
NathanReb
NathanReb approved these changes Mar 26, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@NathanReb NathanReb left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

I just fixed the formatting and added a test to show the before/after the fix.

@NathanReb NathanReb merged commit 04e050c into ocaml-ppx:trunk-support Mar 26, 2024
@anmonteiro anmonteiro deleted the anmonteiro/lexbuf-for-binary-too branch March 26, 2024 22:24
@anmonteiro
Copy link
Copy Markdown
Contributor Author

anmonteiro commented Mar 26, 2024

Thank you!

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

Reviewers

@NathanReb NathanReb NathanReb approved these changes

@ceastlund ceastlund Awaiting requested review from ceastlund ceastlund is a code owner

@panglesd panglesd Awaiting requested review from panglesd panglesd is a code owner

@pitag-ha pitag-ha Awaiting requested review from pitag-ha pitag-ha is a code owner

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

@anmonteiro @NathanReb