proto copier#102
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive example demonstrating the "Proto Copier" feature in Bufrnix, which enables declarative copying of Protocol Buffer files to multiple destinations using various configurations and filtering options.
- Adds proto language support with file copying capabilities to the Bufrnix system
- Implements a new proto copier module with extensive configuration options for file distribution
- Provides a detailed example project showcasing different proto copying scenarios and use cases
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/mkBufrnix.nix | Adds proto language package entry to supported languages |
| src/lib/bufrnix-options.nix | Defines comprehensive configuration options for proto language and copier sub-module |
| src/languages/proto/default.nix | Main proto language module that orchestrates copying operations |
| src/languages/proto/copier.nix | Core copier implementation with file filtering, transformation, and multi-destination support |
| examples/proto-copier/ | Complete example project with README, flake configuration, and sample proto files |
| output/ | Example output files demonstrating the copier's functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if [[ "$proto_file" == ${pattern} ]]; then | ||
| should_exclude=true | ||
| fi |
There was a problem hiding this comment.
The shell pattern matching is incorrect. The variable should be quoted and use proper glob matching syntax. This should be [[ \"$proto_file\" == *${pattern}* ]] or use a case statement for proper pattern matching.
| if [[ "$proto_file" == ${pattern} ]]; then | |
| should_exclude=true | |
| fi | |
| case "$proto_file" in | |
| ${pattern}) should_exclude=true ;; | |
| esac |
| [[ -n "$proto_file" ]] || continue | ||
|
|
There was a problem hiding this comment.
This check for non-empty proto_file is redundant since the same check is performed on line 51. Consider removing this duplicate check to reduce code complexity.
| [[ -n "$proto_file" ]] || continue | |
| target_file="${outputPath}/$rel_path" | ||
|
|
||
| ${optionalString (filePrefix != "") '' | ||
| # Add file prefix | ||
| target_file="${outputPath}/$(dirname "$rel_path")/${filePrefix}$(basename "$rel_path")" | ||
| ''} | ||
|
|
||
| ${optionalString (fileSuffix != "") '' | ||
| # Add file suffix (before extension) | ||
| base_name=$(basename "$rel_path" .proto) | ||
| target_file="${outputPath}/$(dirname "$rel_path")/$base_name${fileSuffix}.proto" | ||
| ''} | ||
|
|
||
| mkdir -p "$target_dir" | ||
| '' else '' | ||
| # Flatten files (copy all to output root) | ||
| file_name=$(basename "$proto_file") | ||
| target_file="${outputPath}/$file_name" | ||
|
|
||
| ${optionalString (filePrefix != "") '' | ||
| # Add file prefix | ||
| target_file="${outputPath}/${filePrefix}$file_name" | ||
| ''} | ||
|
|
||
| ${optionalString (fileSuffix != "") '' | ||
| # Add file suffix (before extension) | ||
| base_name=$(basename "$file_name" .proto) | ||
| target_file="${outputPath}/$base_name${fileSuffix}.proto" | ||
| ''} | ||
| ''} |
There was a problem hiding this comment.
The file prefix logic overrides the target_file variable set on line 71, making that assignment redundant. Consider restructuring to calculate the final target_file path only once to improve clarity.
| target_file="${outputPath}/$rel_path" | |
| ${optionalString (filePrefix != "") '' | |
| # Add file prefix | |
| target_file="${outputPath}/$(dirname "$rel_path")/${filePrefix}$(basename "$rel_path")" | |
| ''} | |
| ${optionalString (fileSuffix != "") '' | |
| # Add file suffix (before extension) | |
| base_name=$(basename "$rel_path" .proto) | |
| target_file="${outputPath}/$(dirname "$rel_path")/$base_name${fileSuffix}.proto" | |
| ''} | |
| mkdir -p "$target_dir" | |
| '' else '' | |
| # Flatten files (copy all to output root) | |
| file_name=$(basename "$proto_file") | |
| target_file="${outputPath}/$file_name" | |
| ${optionalString (filePrefix != "") '' | |
| # Add file prefix | |
| target_file="${outputPath}/${filePrefix}$file_name" | |
| ''} | |
| ${optionalString (fileSuffix != "") '' | |
| # Add file suffix (before extension) | |
| base_name=$(basename "$file_name" .proto) | |
| target_file="${outputPath}/$base_name${fileSuffix}.proto" | |
| ''} | |
| ''} | |
| # Build the target filename with optional prefix and suffix | |
| base_name=$(basename "$rel_path" .proto) | |
| file_name="${base_name}.proto" | |
| if [ -n "${filePrefix}" ]; then | |
| file_name="${filePrefix}${file_name}" | |
| fi | |
| if [ -n "${fileSuffix}" ]; then | |
| file_name="${base_name}${fileSuffix}.proto" | |
| if [ -n "${filePrefix}" ]; then | |
| file_name="${filePrefix}${base_name}${fileSuffix}.proto" | |
| fi | |
| fi | |
| target_file="${target_dir}/${file_name}" | |
| mkdir -p "$target_dir" | |
| '' else '' | |
| # Flatten files (copy all to output root) | |
| base_name=$(basename "$proto_file" .proto) | |
| file_name="${base_name}.proto" | |
| if [ -n "${filePrefix}" ]; then | |
| file_name="${filePrefix}${file_name}" | |
| fi | |
| if [ -n "${fileSuffix}" ]; then | |
| file_name="${base_name}${fileSuffix}.proto" | |
| if [ -n "${filePrefix}" ]; then | |
| file_name="${filePrefix}${base_name}${fileSuffix}.proto" | |
| fi | |
| fi | |
| target_file="${outputPath}/${file_name}" |
| base_name=$(basename "$rel_path" .proto) | ||
| target_file="${outputPath}/$(dirname "$rel_path")/$base_name${fileSuffix}.proto" |
There was a problem hiding this comment.
Similar to the prefix logic, this suffix logic also overrides the target_file variable, potentially overriding any prefix that was applied. The prefix and suffix transformations should be combined into a single calculation to ensure both are applied correctly.
This pull request introduces a comprehensive example for the "Proto Copier" feature in Bufrnix, demonstrating how to declaratively copy Protocol Buffer files to multiple destinations using various configurations. It includes a detailed README, a Nix flake with multiple example scenarios, and a set of realistic proto files organized by use case (API, shared types, external integrations, internal services, and tests). The example is designed to showcase advanced proto distribution patterns, filtering, file transformations, and integration with other language modules.
Key changes:
Documentation & Example Structure
README.mdexplaining the purpose, project structure, available copier configurations, usage instructions, configuration options, output examples, use cases, advanced features, testing steps, integration with other languages, troubleshooting, and links to further reading.Nix Flake Configuration
flake.nixdefining multiple copier scenarios: default copying, advanced multi-destination copying, flattened output with file transformations, selective copying, and combined proto copying with Go code generation. Each scenario demonstrates different filtering and output strategies.Proto File Examples
proto/:api/v1/user_service.proto: Public API for user management.common/v1/types.proto: Shared enums and message types.external/v1/webhook.proto: External-facing webhook service definitions.internal/v1/admin_service.proto: Internal-only admin service, intended to be excluded from public copies.test/test_user.proto: Test utilities, excluded from production outputs.Demonstration Outputs
output/backend/proto/proto/api/v1/user_service.proto) illustrate the result of running the copier with different configurations.These changes provide a robust, real-world demonstration of Bufrnix's proto copier capabilities, serving as both documentation and a testbed for users.
References: