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 parameterized types #5

Merged
merged 10 commits into from
Feb 18, 2025

Conversation

ygdrasil-io
Copy link
Contributor

No description provided.

@ygdrasil-io ygdrasil-io mentioned this pull request Feb 16, 2025
16 tasks
Extended the parser and type model to handle parameterized types such as `sequence`, `record`, `FrozenArray`, and `Promise`. Updated validation logic to recognize these types and parse their parameters correctly. Adjusted the `IdlType` class to store parameterized type information.
Renamed parameters and improved variable naming to enhance readability. Adjusted error messaging to align with updated parameter names for better context.
Previously, all imports were processed without regard for parameterized types, potentially causing issues. This update adds a filter to exclude unsupported parameterized types, improving stability and error handling. A TODO comment is included for future support of these types.
Previously, the tested interface contained 2 attributes, but this has been updated to 7. The change aligns the test with the updated interface definition.
@ygdrasil-io ygdrasil-io force-pushed the feature/add-parameterized-types branch from 6cd5199 to 5def144 Compare February 16, 2025 21:50
Corrected inconsistent indentation in the lambda to improve code readability. This change does not alter functionality but ensures the code adheres to proper formatting standards.
Removed unnecessary conditional block checking for parameterized types within angled brackets. Simplifies code by consolidating checks and reducing duplication, improving maintainability.
@ygdrasil-io ygdrasil-io marked this pull request as ready for review February 16, 2025 22:00
}

val typeParams = typeName.substring(typeName.indexOf("<") + 1, typeName.lastIndexOf(">"))
typeName = typeName.substring(0, typeName.indexOf("<"))
Copy link
Owner

Choose a reason for hiding this comment

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

Personally, I prefer using the substringBefore / substringAfter functions for these kind of things. E.g. something like this:

    val typeParams = typeName.substringAfter('<').substringBeforeLast('>')
    typeName = typeName.substringBefore('<')

But that's only a personal preference so feel free to keep it this 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 agree, that part lack also of Unit Test on the result, working on it.

Refactor type parameter extraction to use more concise Kotlin functions `substringAfter` and `substringBeforeLast`.
@ygdrasil-io ygdrasil-io marked this pull request as draft February 18, 2025 10:40
Updated `IdlType.parameterType` from a single string to a list to handle multiple type parameters. Adjusted parsing logic in `WebIdlStream` and updated tests to validate the new structure. Ensures compatibility with complex type definitions like `record` and `FrozenArray`.
Renamed the `parameterType` field to `parameterTypes` in the `IdlType` class for consistency and clarity. Updated relevant test assertions to reflect this change.
@ygdrasil-io ygdrasil-io marked this pull request as ready for review February 18, 2025 12:09
@fabmax fabmax merged commit 9c62da4 into fabmax:main Feb 18, 2025
2 checks passed
@ygdrasil-io ygdrasil-io deleted the feature/add-parameterized-types branch February 18, 2025 20:35
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