Skip to content

Conversation

@teowelton
Copy link

Closes #145

As per the comment in issue #145, marginally change the API to pass envs through .read_file() and .parse(), then parse file contents using that context.

@evmar
Copy link
Owner

evmar commented Oct 31, 2025

I think this needs an update to ab59736 (which I think you need to rebase onto)

@teowelton
Copy link
Author

teowelton commented Oct 31, 2025

I believe my repo is already up to date with that commit?

Screenshot 2025-10-31 at 09 38 16

};

let mut combined_envs: Vec<&dyn eval::Env> = vec![&parser.vars];
combined_envs.extend(envs);
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like you never use this (?)

Copy link
Owner

@evmar evmar left a comment

Choose a reason for hiding this comment

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

If you fixed the issue, the test I mentioned should be affected by this change. It has the fixed code commented out too

@teowelton
Copy link
Author

Oh my apologies, the unused variable was an oversight, I must've accidentally undone some changes, and I thought the testing case was already testing for the correct behavior.

I'll push a fix soon!

@teowelton
Copy link
Author

Closing this since my fix didn't resolve the issue, and I'm going to be busy for a while. @olivia-banks will submit a new PR with a working solution soon!

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.

Variables not Expanded in Included Fragment

2 participants