Conversation
1454b74 to
03831bc
Compare
|
Hi @ndeloof — this fixes the FIXME you left in |
The tty, plain, and json EventProcessor constructors accepted no dry-run flag, so the DRY-RUN prefix was never rendered even when --dry-run was passed. Signed-off-by: hiroto.toyoda <hiroto.toyoda@dena.com>
04e5074 to
b8d86e5
Compare
|
Hi @ndeloof @glours 👋 Gentle nudge on this one when you have a moment. Rebased on the new tty model/layout/screen split, and reworked along the way: Verified against a real daemon with a 2-service project:
If you don't think this is worth merging, just let me know and I'll close it. |
What I did
docker compose --dry-run ... never rendered the DRY-RUN MODE prefix in progress output, on any of the three renderers.
The pieces are there but nothing connects them:
TTY: termWriter has a dryRun field and tty_layout.go prefixes every row with it, and WithDryRun() exists as a TermOption — but nothing ever passes that option, so it is always false.
plain / json: plainWriter/jsonWriter both have a dryRun field used at render time, but neither Plain() nor JSON() accepts a value for it, so it is always false too.
This wires the already-parsed --dry-run flag through selectEventProcessor to all three:
TTY: passes the existing display.WithDryRun() option — cmd/display/tty.go itself is unchanged.
plain / json: adds a dryRun bool parameter to the two constructors.
Tests: the TTY case asserts on the rendered frame (the marker actually reaches the output) rather than on the field; plain_test.go / json_test.go cover the other two renderers.
Note: this PR was originally written before the tty.go model/layout/screen refactor and has been rebased and reworked on top of it. The earlier version added a dryRun parameter to display.Full(); it now uses the WithDryRun() option that the refactor introduced, which keeps tty.go untouched.
Related issue
N/A
(not mandatory) A picture of a cute animal, if possible in relation to what you did
🐈