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

fix[ux]: include imported structs in interface output #4362

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

sandbubbles
Copy link
Collaborator

What I did

Included structs from imported modules in the -f interface output. Fix the #4298 issue.

How I did it

Added a function that recursively walks imported modules and collects structs. It contains visited set to track which imports were already handled.

How to verify it

Added test cases and previous test cases work.

Commit message

Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@sandbubbles
Copy link
Collaborator Author

sandbubbles commented Nov 16, 2024

Should the name of a struct include module name, id?

@charles-cooper charles-cooper marked this pull request as ready for review November 23, 2024 20:35
@@ -159,6 +161,19 @@ def build_interface_output(compiler_data: CompilerData) -> str:
return out


def _get_structs(m: ModuleT, visited: set[ModuleT] = None) -> [StructT]:
Copy link
Member

Choose a reason for hiding this comment

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

we can use ModuleT.reachable_imports here instead of implementing the recursion

if m in visited:
return []
visited.add(m)
structs = list(m.interface.structs.values())
Copy link
Member

Choose a reason for hiding this comment

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

hmm. what if two modules have a struct with the same name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, that's what i was getting at with #4362 (comment), I'll prepend it.

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