restructure Pi tool formatting into per-method dispatch with generic fallback#1012
restructure Pi tool formatting into per-method dispatch with generic fallback#1012LasmarKhalifa wants to merge 1 commit into
Conversation
2390b15 to
3f04dcc
Compare
3f04dcc to
58fc38b
Compare
mathiusj
left a comment
There was a problem hiding this comment.
Foundation looks clean — per-method dispatch with respond_to?(..., true) and a generic fallback reads well, and the test rewrite is a nice tightening. A few things to consider before the whole stack lands on top of this.
| # DEPLOY | ||
| # | ||
| #: () -> String | ||
| def format_unknown |
There was a problem hiding this comment.
Robustness: dispatch trusts arguments is a well-formed Hash. format_unknown calls arguments.empty?/.map, and downstream formatters do arguments.values_at(...) / arguments[:edits].length. If Pi ever sends a non-Hash (or nil) arguments, or a formatter's typed assumption is violated (e.g. :edits as a String), dispatch raises inside the progress logger. Since this is display-only, the highest-leverage fix lives here in the foundation: wrap the send(format_method_name) in a rescue that falls back to format_unknown. That protects all 7 downstream PRs at once and means per-tool reviewers don't have to scrutinize each formatter's arg assumptions. (Normalizing @arguments to {} when non-Hash in initialize is a cheap complement.)
There was a problem hiding this comment.
Added the fallback!
58fc38b to
040a794
Compare

Closes #1022
Builds the foundation for the stack by replacing the current tool formatter routing to a per-method dispatch.