add grep tool formatters#1019
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. |
LasmarKhalifa
force-pushed
the
07-02/pi-tool-formatter-edit
branch
from
2d3ce63 to
7730ee2
Compare
LasmarKhalifa
force-pushed
the
07-02/pi-tool-formatter-grep
branch
2 times, most recently
from
3b64e5c to
10a0676
Compare
LasmarKhalifa
force-pushed
the
07-02/pi-tool-formatter-edit
branch
from
7730ee2 to
676a57b
Compare
LasmarKhalifa
force-pushed
the
07-02/pi-tool-formatter-grep
branch
from
10a0676 to
3799ee2
Compare
LasmarKhalifa
force-pushed
the
07-02/pi-tool-formatter-grep
branch
from
3799ee2 to
919a175
Compare
LasmarKhalifa
requested review from
dersam,
juniper-shopify,
mathiusj and
nfgrep
as code owners
mathiusj
left a comment
mathiusj
left a comment
There was a problem hiding this comment.
grep formatters are the most heuristic in the stack — a couple of notes.
| #: () -> String | ||
| def format_grep | ||
| pattern, path, glob, limit = arguments.values_at(:pattern, :path, :glob, :limit) | ||
| base = "GREP \"#{truncate(pattern)}\"" |
There was a problem hiding this comment.
The pattern is truncated but path, glob, and limit are appended untruncated, and they stack: GREP "<45>" <full path> (glob: <full glob>, limit: <n>) can easily run 150+ chars, contradicting the "generally one line" goal stated in the foundation comments. If the one-line bound matters, truncate the composed result; otherwise align the comment wording.
| #: () -> String | ||
| def format_grep | ||
| lines = content.to_s.lines.map(&:strip).reject(&:empty?) | ||
| matches, notes = lines.partition { |line| line.match?(%r{\A\S+/}) || line.match?(/\A(?:\S+:)?\d+:/) } |
There was a problem hiding this comment.
Match/note heuristic is best-effort and will miscount. %r{\A\S+/} treats any line starting with a non-space run + / as a match, and \A(?:\S+:)?\d+: as a line-number hit. Real grep hits that don't carry a path/line prefix (e.g. grep -h, or matched content that doesn't fit either shape) get bucketed as notes and undercount; conversely a note containing an early / counts as a match. Since "3 matches" is shown to the user, a wrong count is a visible lie. It won't break anything, but please add a comment that this depends on Pi's grep output format and is intentionally approximate — so nobody later trusts the count as exact or "fixes" it by tightening the regex without knowing the contract.
2 participants
Closes #1027