Skip to content

Implement support for Rust features - #22

Merged
Calsign merged 2 commits into
Calsign:mainfrom
mrkkrp:features
Jan 6, 2026
Merged

Implement support for Rust features#22
Calsign merged 2 commits into
Calsign:mainfrom
mrkkrp:features

Conversation

@mrkkrp

@mrkkrp mrkkrp commented Dec 14, 2025

Copy link
Copy Markdown
Contributor

Close #21.

@mrkkrp
mrkkrp force-pushed the features branch 4 times, most recently from ef84d6c to 9d79497 Compare December 16, 2025 13:23
@mrkkrp
mrkkrp marked this pull request as ready for review December 16, 2025 13:23
@mrkkrp
mrkkrp force-pushed the features branch 2 times, most recently from 6c1cc3d to 45fd3f1 Compare December 16, 2025 13:32
@mrkkrp

mrkkrp commented Dec 16, 2025

Copy link
Copy Markdown
Contributor Author

@Calsign I believe this is ready for review. The readme is probably the best starting point to get an overview of the design.

@mrkkrp mrkkrp mentioned this pull request Dec 16, 2025
Comment thread README.md Outdated
@mrkkrp

mrkkrp commented Jan 5, 2026

Copy link
Copy Markdown
Contributor Author

I have rebased the PR, adjusted the readme as you request, and attempted to make sure that

gazelle_rust reads the set of features in the crate_features attribute and skips adding dependencies which are not needed for the enabled features.

holds even in the pure Bazel mode (no tests for this though, please take a look at the implementation).

@Calsign Calsign left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This looks good.

I might like to see generation tests for the following cases:

  • A negative rust_feature directive removing a feature from the set of default features
  • A nested package structure where directives are used to override behavior from a parent package

Comment thread rust_parser/main.rs Outdated
match args {
Args::OneShot { path } => {
let mut rust_imports = parser::parse_imports(path)?;
let mut rust_imports = parser::parse_imports(path, &Vec::new())?;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nit: use &[] instead of &Vec::new()

Comment thread rust_parser/parser.rs
Comment thread rust_parser/parser.rs Outdated
Comment thread rust_parser/parser.rs Outdated
Comment thread rust_language/lang.go Outdated
@mrkkrp

mrkkrp commented Jan 6, 2026

Copy link
Copy Markdown
Contributor Author

@Calsign Please take another look, I think I have addressed all your comments in 8e52e30.

@Calsign

Calsign commented Jan 6, 2026

Copy link
Copy Markdown
Owner

Thanks!

@Calsign
Calsign merged commit 921986e into Calsign:main Jan 6, 2026
1 check passed
@mrkkrp
mrkkrp deleted the features branch January 28, 2026 13:48
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.

Support for Rust's features

2 participants