Support distribution of RPL patterns#108
Support distribution of RPL patterns#108jellllly420 wants to merge 1 commit intoRPL-Toolchain:masterfrom
Conversation
jellllly420
force-pushed
the
feat/publish
branch
3 times, most recently
from
3847f55 to
7764665
Compare
jellllly420
force-pushed
the
feat/publish
branch
from
7764665 to
270fa12
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for distributing RPL patterns through Cargo's infrastructure by introducing a new rpl_config crate that handles configuration loading and pattern resolution. The implementation enables users to specify pattern groups via --patterns command-line arguments, which can reference both local and remote (published crate) pattern groups.
Changes:
- Added new
rpl_configcrate to handle configuration parsing and pattern resolution - Extended command-line argument parsing to support
--patternsand--manifest-pathoptions - Implemented pattern group resolution for both local and remote (crate-based) pattern sources
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.rs | Added parsing for --patterns and --manifest-path arguments, integrated config loading and environment variable setup |
| crates/rpl_config/src/lib.rs | Main entry point for config loading, defines error types and public Config struct |
| crates/rpl_config/src/patterns.rs | Core pattern resolution logic for local and remote groups, handles deduplication and path resolution |
| crates/rpl_config/src/run.rs | Handles loading of runtime configuration options like inline-mir |
| crates/rpl_config/src/util.rs | Utility functions for base directory resolution and config file reading |
| crates/rpl_config/Cargo.toml | Dependencies for the new rpl_config crate |
| Cargo.toml | Workspace configuration to include rpl_config crate |
| Cargo.lock | Lock file updates for new dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if let Some(value) = arg.strip_prefix("--patterns=") { | ||
| pattern_groups.push(value.to_string()); | ||
| continue; | ||
| } | ||
| if arg == "--patterns" { | ||
| if let Some(value) = iter.next() { | ||
| pattern_groups.push(value); | ||
| } else { | ||
| pattern_groups.push(String::new()); | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
The new functionality for parsing --patterns arguments lacks test coverage. The existing test suite in src/main.rs includes tests for --fix and --no-deps flags (lines 252-281). Consider adding similar tests to verify: (1) parsing of --patterns=<value> format, (2) parsing of --patterns <value> format, (3) handling of multiple --patterns arguments, (4) behavior when --patterns is provided without a value, and (5) correct population of the pattern_groups field.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
Prefer to test in this PR instead. Asked Codex to add some test. Let's see if it works.
jellllly420
force-pushed
the
feat/publish
branch
from
270fa12 to
36dfaf3
Compare
jellllly420
force-pushed
the
feat/publish
branch
from
36dfaf3 to
56e2fff
Compare
| if config.is_none() { | ||
| if selected_groups.is_empty() { | ||
| return Ok(None); | ||
| } | ||
| if selected_groups.iter().all(|name| is_remote_spec(name)) { | ||
| return Ok(Some(resolve_remote_selection(manifest_path, selected_groups)?)); | ||
| } | ||
| return Err(ConfigError::ConfigNotFound { path: config_path }); | ||
| } | ||
|
|
||
| let config = config.unwrap(); |
There was a problem hiding this comment.
I guess we can use let ... else here to avoid unwrap.
2 participants
Yet to test. DO NOT merge for now please.This PR enables distribution of RPL patterns over Cargo's infrastructure, which further decouples the definition and detection of RPL patterns.