Remove the dead format keyword from Grape::Router::Pattern#2782
Merged
Conversation
`Pattern#initialize` took a `format:` keyword and, in `extract_capture`, constrained the mustermann `:format` capture with it. But nothing ever passed a non-nil value: the endpoint stopped passing `format:` when the vestigial `Grape::Endpoint::Options` `format` member was removed (#2779), and grape-swagger's helper passes `format: nil`. It was also redundant — a route's `:format` capture comes from the path suffix built by `Grape::Path` (`(.json)` for a specific format, `(.:format)` otherwise), not from the pattern. Drop the keyword and the dead capture branch. `DEFAULT_CAPTURES` still lists `format` (the path-derived capture) and `map_str` still serves `version`, so matching is unchanged. `Grape::Router::Pattern.new(..., format: …)` now raises `unknown keyword: :format`; corrected the UPGRADING note that previously described it as an optional keyword. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ericproulx
force-pushed
the
remove_pattern_format
branch
from
4956ab9 to
5b62850
Compare
Danger ReportNo issues found. |
ericproulx
added a commit
that referenced
this pull request
Jul 5, 2026
`version`, `anchor` and `requirements` are the attributes that shape the matcher — they are passed to `Grape::Router::Pattern` to build the mustermann pattern. They were also passed a second time to `Grape::Router::Route` and stored as ivars, purely to back the readers. Expose them on `Pattern` instead (`attr_reader`) and delegate the `BaseRoute` readers to `@pattern`, alongside the existing `path`/`origin` delegation. `to_routes` no longer passes them to `Route.new`. Metadata that does not shape matching — `namespace`, `prefix`, `settings` — stays as route keyword arguments. grape-swagger already passes these to `Pattern.new` and none of them to `Route.new`, so its routes' readers become correct rather than `nil`. Also collapse `extract_capture` to a guard plus a single merge. With the `format` keyword gone (#2782) it just adds an optional `version` constraint to `requirements`, so the intermediate accumulator was noise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ericproulx
added a commit
that referenced
this pull request
Jul 6, 2026
`version`, `anchor` and `requirements` are the attributes that shape the matcher — they are passed to `Grape::Router::Pattern` to build the mustermann pattern. They were also passed a second time to `Grape::Router::Route` and stored as ivars, purely to back the readers. Expose them on `Pattern` instead (`attr_reader`) and delegate the `BaseRoute` readers to `@pattern`, alongside the existing `path`/`origin` delegation. `to_routes` no longer passes them to `Route.new`. Metadata that does not shape matching — `namespace`, `prefix`, `settings` — stays as route keyword arguments. grape-swagger already passes these to `Pattern.new` and none of them to `Route.new`, so its routes' readers become correct rather than `nil`. Also collapse `extract_capture` to a guard plus a single merge. With the `format` keyword gone (#2782) it just adds an optional `version` constraint to `requirements`, so the intermediate accumulator was noise. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Grape::Router::Pattern#initializetook aformat:keyword and used it inextract_captureto constrain the mustermann:formatcapture. It was dead:nilvalue. The endpoint stopped passingformat:when the vestigialGrape::Endpoint::Optionsformatmember was removed (Promote route metadata to Route keyword arguments #2779), and grape-swagger's test helper passesformat: nil. Soformat.present?was never true andcapture[:format] = …never ran.:formatcapture comes from the path suffix thatGrape::Pathbuilds —(.json)for a specific format,(.:format)otherwise — not from the pattern.This drops the keyword and the dead branch.
DEFAULT_CAPTURESstill listsformat(that path-derived capture) andmap_strstill servesversion, so matching is unchanged —format :jsonstill yields/x(.json), plain/x(.:format).Upgrading
Grape::Router::Pattern.new(..., format: …)now raisesunknown keyword: :format. The UPGRADING note from #2781 (which describedformat:as an optional keyword onPattern) is corrected accordingly.Note: grape-swagger's test helper passes
format: niltoPattern.newand will need a one-line drop — but it already can't run against grape 4.0 HEAD (it callsRoute.newwithout the requiredforward_match:), so this folds into the 4.0 update it needs regardless.🤖 Generated with Claude Code