add edit tool formatters#1018
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
852ae68 to
f2399f0
Compare
2d3ce63 to
7730ee2
Compare
f2399f0 to
4166e5d
Compare
7730ee2 to
676a57b
Compare
mathiusj
left a comment
There was a problem hiding this comment.
edit formatters look good. Two small things.
| #: () -> String | ||
| def format_edit | ||
| path = arguments[:path] | ||
| edits = arguments[:edits] || [] |
There was a problem hiding this comment.
|| [] only guards nil — if :edits ever arrives as a non-Array (e.g. a String or a single Hash), .length either silently returns the wrong count (char count for a String) or raises. Given this is display-only, I wouldn't add validation here; the clean fix is a rescue→format_unknown at the dispatch site in #1012 (commented there). Just flagging so it's a conscious choice rather than an assumption. Also note EDIT #{path} interpolates the path untruncated, so a long path blows past the one-line goal the comments state.
| # EDIT OK | ||
| # | ||
| #: () -> String | ||
| def format_edit |
There was a problem hiding this comment.
Readability nit: the result formatter reads @input[:path] while the call formatter (same field) reads arguments[:path]. Both correct — @input is the resolved call's args in the result context, arguments is the call's own ivar — but the naming divergence across the two files costs a beat to reconcile. A one-line comment noting they're deliberately different objects would help future readers.

Closes #1026