feat: add --fields and --max-lines global CLI options (DIS-145)#38
feat: add --fields and --max-lines global CLI options (DIS-145)#38
Conversation
Add --fields <fields> to select specific JSON fields in output.
Add --max-lines <lines> to truncate output with line count indicator.
Uses Commander preAction hook and module-level output options to avoid
changing any command files. Field filtering handles plain objects,
arrays, and wrapped API responses (e.g. {nfts: [...], next: ...}).
Truncation appends '... (N more lines)' indicator.
Co-Authored-By: Chris K <ckorhonen@gmail.com>
Original prompt from Chris K |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
- filterFields now checks if any requested field matches a top-level key.
If yes, picks from top level (handles Collection with array props like
contracts, editors). If no match, treats as wrapper and filters array
items (handles {nfts: [...], next: ...} responses).
- Changed maxLines guard from truthiness to != null so --max-lines 0 works.
- Added tests for objects with array properties and maxLines 0.
Co-Authored-By: Chris K <ckorhonen@gmail.com>
ckorhonen
left a comment
There was a problem hiding this comment.
Code Review
Overall: Approve with suggestions — All 162 tests pass. E2E confirmed both --fields and --max-lines work correctly.
Issues Found
-
Wrapper-detection heuristic is fragile —
filterFieldschecks for any array-valued key to decide if an object is a "wrapper." This misidentifies real API objects likeCollection(hascontracts,editors,feesarrays) andNFT(hastraits,ownersarrays). Running--fields name,collectionon a Collection object would filter inside those sub-arrays instead of the top-level object. Fix: remove wrapper detection entirely — always applypickFieldsat the top level. -
setOutputOptions/OutputOptionsshouldn't be in the SDK barrel export — These are CLI-internal concerns. Module-level mutable state exported to SDK consumers creates hidden coupling, state leakage, and concurrency hazards. -
Missing
--max-linesvalidation —--max-lines 0truncates everything,--max-lines -1silently drops the last line viaslice(0, -1). Add validation that the value must be >= 1.
Minor Notes
- Truncated JSON output is no longer valid JSON — worth documenting in
--help --fieldsonly works on top-level keys — worth documenting this limitationpickFieldsoutput key order follows user's--fieldsorder, not original order — arguably a feature but undocumented
I've pushed a commit addressing issues 1-3.
- Remove wrapper-detection heuristic from filterFields; always apply pickFields at top level (issue #1 from review) - Remove setOutputOptions/OutputOptions from SDK barrel export to avoid state leakage for SDK consumers (issue #2 from review) - Add --max-lines validation requiring >= 1 (issue #3 from review) - Update tests to match simplified filterFields behavior Co-Authored-By: Chris K <ckorhonen@gmail.com>
|
Addressed all 3 issues from the code review in c8df287:
163 tests passing, lint/typecheck clean. |
feat: add --fields and --max-lines global CLI options (DIS-145)
Summary
Adds two global CLI options to control output size, motivated by agents piping CLI output through
python3 -m json.tool | headwhich caused a 30-minute false-401 debugging incident.--fields <fields>— Comma-separated list of fields to include in output. AppliespickFieldsat the top level of the response object (or each item if the response is an array). For paginated wrapper responses like{nfts: [...], next: "cursor"}, users specify the wrapper keys they want (e.g.--fields nfts,next).--max-lines <lines>— Truncates formatted output after N lines, appending... (N more lines). Must be >= 1.Uses Commander's
preActionhook with module-level state inoutput.tsso zero command files were modified — all 9 existing commands get both options for free.Updates since last revision
filterFieldsnow always appliespickFieldsat the top level. No more fragile heuristic that misidentified objects likeCollection(with array properties) as paginated wrappers.setOutputOptions/OutputOptionsfrom SDK barrel export (per code review): These are CLI-internal concerns; exporting mutable module-level state to SDK consumers created hidden coupling and state leakage risk.--max-linesvalidation (per code review): Values < 1 are rejected withprocess.exit(2). Removed themaxLines=0edge case.maxLinesnullish check (flagged by Devin Review): Changedif (_outputOptions.maxLines)toif (_outputOptions.maxLines != null).Review & Testing Checklist for Human
--fieldsUX for paginated responses: With the heuristic removed,--fields identifier,nameon a{nfts: [...], next: "..."}response returns{}(no matching top-level keys). Users must use--fields nfts,nextto select wrapper keys. Verify this is the intended UX.formatOutputdirectly, not the Commander argv parsing pipeline. Verify with real commands:--max-linesappends a... (N more lines)suffix. Worth considering whether this should be documented in--help.Notes