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 functions for getting all sources used in Flowscript Compilation #87

Merged
merged 4 commits into from
Nov 30, 2024

Conversation

AnimatedSwine37
Copy link
Contributor

This PR adds two functions TryGetImports and a TryCompileWithImports overload that returns a list of all used source files as an out parameter.
I intend to use both with BF Emulator/Persona Essentials so I can take into account changes in imports to flowscript files when caching compiled bf files.

Copy link
Owner

@tge-was-taken tge-was-taken left a comment

Choose a reason for hiding this comment

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

A bit hacky, but I like the idea.

/// <returns>True if imports could be resolved, false otherwise</returns>
public bool TryGetImports(List<string> files, out string[] resolvedImports)
{
CompilationUnit compilationUnit = new CompilationUnit();
Copy link
Owner

Choose a reason for hiding this comment

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

Would it be a lot of work to refactor to get rid of the dummy compilation unit (using just an array of Imports)?
Would be a bit cleaner that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how hard it'd be but I'll give it a try.
This was definitely me being a bit lazy and just wanting something that worked quickly 😅

Copy link
Owner

Choose a reason for hiding this comment

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

Any updates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry not yet. I did a bit when you initially asked and realised it's going to be more work than I first thought. Nothing I can't do but I think it'll take a couple of hours which I just haven't found yet.

I'll try and do it this weekend so we can close this off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That wasn't too hard actually.

I've changed TryGetImports so it now uses a stripped down version of TryResolveImports which uses a list of Imports rather than a CompilationUnit.
It also now only parses flowscript files since they can contain additional imports whereas compiled bfs and messages can't so parsing the actual files is just a waste of time.

@tge-was-taken
Copy link
Owner

Thanks. Looks good.

@tge-was-taken tge-was-taken merged commit e4e50b5 into tge-was-taken:master Nov 30, 2024
1 check passed
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.

2 participants