Skip to content

feat: add include support to IDL v2#1175

Merged
m62624 merged 6 commits intomasterfrom
feat/idl-v2-alias-include
Mar 27, 2026
Merged

feat: add include support to IDL v2#1175
m62624 merged 6 commits intomasterfrom
feat/idl-v2-alias-include

Conversation

@m62624
Copy link
Copy Markdown
Member

@m62624 m62624 commented Jan 13, 2026

No description provided.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for type aliases and !@include directives in the IDL, which is a great feature enhancement. The changes are comprehensive, touching the parser, AST, client generator, and templates to accommodate these new features. A new preprocessor is also added to handle the include logic. The implementation is solid, but I've identified a bug in the client code generator that results in a trailing comma in generic type parameter lists. I've also noted a potential robustness issue in the preprocessor's logic for detecting top-level scope, which could be improved for future maintainability. Overall, great work on this feature.

Comment thread rs/idl-parser-v2/src/preprocess.rs Outdated
@m62624 m62624 marked this pull request as ready for review January 13, 2026 07:04
@m62624 m62624 requested a review from vobradovich January 13, 2026 07:05
@m62624 m62624 marked this pull request as draft February 17, 2026 07:18
@m62624 m62624 force-pushed the feat/idl-v2-alias-include branch from 6bbc475 to 1f53307 Compare February 17, 2026 07:57
@m62624 m62624 marked this pull request as ready for review March 4, 2026 05:46
Base automatically changed from master-idl-v2 to master March 17, 2026 13:00
@m62624 m62624 force-pushed the feat/idl-v2-alias-include branch from 77d1f31 to 905e4fa Compare March 26, 2026 05:46
@m62624 m62624 changed the title feat: support type aliases and includes in IDL feat: add include support to IDL v2 Mar 26, 2026
@gear-tech gear-tech deleted a comment from gemini-code-assist Bot Mar 26, 2026
@m62624
Copy link
Copy Markdown
Member Author

m62624 commented Mar 26, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces an IDL preprocessor in sails-idl-parser-v2 to support !@include directives, enabling modular IDL definitions and preventing duplicate inclusions through content hashing. The client-gen crate has been updated to utilize this new preprocessor. Feedback suggests refining the brace_level handling in the preprocessor for better syntax error management and improving the test IdlLoader implementation to use content hashing for more robust uniqueness checks and path resolution.

Comment thread rs/idl-parser-v2/src/preprocess.rs Outdated
Comment thread rs/idl-parser-v2/src/preprocess.rs
@m62624 m62624 force-pushed the feat/idl-v2-alias-include branch from 7944995 to 028c856 Compare March 26, 2026 07:47
Comment thread rs/idl-parser-v2/src/preprocess.rs Outdated
Comment thread rs/idl-parser-v2/src/preprocess.rs Outdated
Comment thread rs/idl-parser-v2/src/preprocess.rs Outdated
Comment thread rs/idl-parser-v2/src/preprocess.rs Outdated
Comment thread rs/idl-parser-v2/src/lib.rs
@m62624 m62624 requested a review from vobradovich March 26, 2026 12:46
@m62624 m62624 force-pushed the feat/idl-v2-alias-include branch from 3f0b8b6 to 0b72afb Compare March 26, 2026 12:49
@m62624 m62624 force-pushed the feat/idl-v2-alias-include branch from 0b72afb to f7d4d7f Compare March 26, 2026 13:12
@m62624 m62624 merged commit 4555a59 into master Mar 27, 2026
4 checks passed
@m62624 m62624 deleted the feat/idl-v2-alias-include branch March 27, 2026 11:22
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