Enhance array index handling and update RBS configuration#16
Merged
Conversation
…ation for index base
…ation for index base
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a global StructuredParams configuration option to control whether array indices shown in human_attribute_name/error messages are displayed as 0-based (default) or 1-based, with corresponding docs, tests, and type signature updates.
Changes:
- Introduce
StructuredParams.configure/configuration/reset_configuration!and thearray_index_basesetting. - Apply
array_index_basewhen formatting array indices in nested human attribute names (including i18n templates). - Update documentation, RBS signatures, and RuboCop/Steep configuration; refresh
rbs_collectionlockfiles.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/structured_params.rb | Adds global configuration API and array_index_base setting. |
| lib/structured_params/i18n.rb | Uses configured index base when rendering array indices in labels/i18n. |
| spec/i18n_spec.rb | Adds specs for 0-based vs 1-based behavior and validation. |
| docs/installation.md | Documents the new configuration option with examples. |
| sig/structured_params.rbs | Updates public RBS API for configuration methods/options. |
| sig/structured_params/i18n.rbs | Updates i18n module documentation in RBS. |
| sig/structured_params/type/array.rbs | Clarifies permit_attribute_names behavior in RBS docs. |
| .rubocop.yml | Adjusts RuboCop method length threshold. |
| .rubocop_rbs.yml | Updates rubocop-rbs_inline exclusions. |
| Steepfile | Suppresses library RBS diagnostic noise from gem_rbs_collection. |
| rbs_collection.yaml | Minor YAML cleanup. |
| rbs_collection.lock.yaml | Updates locked RBS collection revisions/dependencies. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5984949506
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a global Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 7 minutes and 11 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.rubocop.yml (1)
33-34: ⚡ Quick winPrefer local cop suppression over a global
MethodLengthrelaxation.Raising this globally weakens method-size guardrails across the whole codebase for a single hotspot. Keep the project default stricter and suppress only where needed.
Proposed change
Metrics/MethodLength: - Max: 25 + Max: 20# In lib/structured_params/i18n.rb, around build_nested_label # rubocop:disable Metrics/MethodLength def build_nested_label(result, index, attr_human, options) # ... end # rubocop:enable Metrics/MethodLength🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.rubocop.yml around lines 33 - 34, Don't relax Metrics/MethodLength globally; instead keep the Max: 25 default and add a local suppression around the long method. Edit the long method build_nested_label in lib/structured_params/i18n.rb and wrap it with a rubocop disable/enable pair (rubocop:disable Metrics/MethodLength ... rubocop:enable Metrics/MethodLength) so only build_nested_label is exempt while the global cop remains strict..rubocop_rbs.yml (1)
12-15: ⚡ Quick winScope the RBS inline exclusion more narrowly.
Excluding the entire file reduces type-safety signal for future additions in
lib/structured_params.rb. Prefer targeted inline disable only where the cop misfires.Proposed change
Style/RbsInline/UntypedInstanceVariable: Exclude: - 'lib/structured_params/params.rb' - - 'lib/structured_params.rb'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.rubocop_rbs.yml around lines 12 - 15, Remove the broad file-level exclusion for the Style/RbsInline/UntypedInstanceVariable cop and instead scope the suppression to the exact code locations that trigger false positives: delete the 'lib/structured_params.rb' entry from the Exclude list in the RuboCop RBS config and add targeted inline disables (e.g. rubocop:disable Style/RbsInline/UntypedInstanceVariable / rubocop:enable ...) around the specific untyped instance-variable declarations inside the source file; keep or retain the existing exclusion for structured_params/params only if that file truly needs a full-file exemption.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.rubocop_rbs.yml:
- Around line 12-15: Remove the broad file-level exclusion for the
Style/RbsInline/UntypedInstanceVariable cop and instead scope the suppression to
the exact code locations that trigger false positives: delete the
'lib/structured_params.rb' entry from the Exclude list in the RuboCop RBS config
and add targeted inline disables (e.g. rubocop:disable
Style/RbsInline/UntypedInstanceVariable / rubocop:enable ...) around the
specific untyped instance-variable declarations inside the source file; keep or
retain the existing exclusion for structured_params/params only if that file
truly needs a full-file exemption.
In @.rubocop.yml:
- Around line 33-34: Don't relax Metrics/MethodLength globally; instead keep the
Max: 25 default and add a local suppression around the long method. Edit the
long method build_nested_label in lib/structured_params/i18n.rb and wrap it with
a rubocop disable/enable pair (rubocop:disable Metrics/MethodLength ...
rubocop:enable Metrics/MethodLength) so only build_nested_label is exempt while
the global cop remains strict.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3353fe63-a24a-48c9-8524-b3cc47a89ef1
📒 Files selected for processing (12)
.rubocop.yml.rubocop_rbs.ymlSteepfiledocs/installation.mdlib/structured_params.rblib/structured_params/i18n.rbrbs_collection.lock.yamlrbs_collection.yamlsig/structured_params.rbssig/structured_params/i18n.rbssig/structured_params/type/array.rbsspec/i18n_spec.rb
💤 Files with no reviewable changes (1)
- rbs_collection.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new global configuration option to the
StructuredParamsgem, allowing users to control whether array indices in human attribute names and error messages are displayed as 0-based (default) or 1-based (human-friendly). The change is fully documented, covered by tests, and reflected in both code and type signatures. Additional improvements include minor RuboCop and type checking configuration updates, and dependency lockfile updates.New Configuration Option and Usage:
StructuredParams.configureinterface with aarray_index_basesetting (default: 0, can be set to 1) to control whether array indices in error messages and human attribute names are 0-based or 1-based. This affects both API param error messages and form object full messages. [1] [2]docs/installation.md) to explain the new configuration option, including usage examples and option descriptions. [1] [2]i18n and Human Attribute Name Behavior:
lib/structured_params/i18n.rband its type signatures to use the configuredarray_index_basewhen displaying array indices in human attribute names and error messages, including in i18n templates. Documentation and inline comments have been updated accordingly. [1] [2] [3] [4] [5] [6] [7]Type Signatures and RuboCop/Steep Configuration:
lib/structured_params.rbto RuboCop RBS exclusions and increased the allowed method length in.rubocop.yml. Suppressed errors from broken library RBS files inSteepfile. [1] [2] [3]Dependency and Lockfile Updates:
rbs_collection.lock.yamlto newer gem_rbs_collection revisions and made minor dependency version adjustments. [1] [2] [3] [4] [5] [6]Other Improvements:
permit_attribute_namesinType::Arrayfor clarity.These changes make StructuredParams more flexible and user-friendly, especially for applications targeting end-users who expect 1-based numbering in UI and error messages.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores