diff --git a/js/.changeset/fix-visual-continuity.md b/js/.changeset/fix-visual-continuity.md new file mode 100644 index 0000000..acd74af --- /dev/null +++ b/js/.changeset/fix-visual-continuity.md @@ -0,0 +1,26 @@ +--- +'start-command': patch +--- + +Fix visual continuity in docker isolation mode (#73) + +The empty line is now properly placed after the command line (e.g., `$ docker pull alpine:latest`) +instead of before it, maintaining consistent visual structure in the timeline output. + +Before: + +``` +│ + +$ docker pull alpine:latest +latest: Pulling from library/alpine +``` + +After: + +``` +│ +$ docker pull alpine:latest + +latest: Pulling from library/alpine +``` diff --git a/js/src/bin/cli.js b/js/src/bin/cli.js index 00ff2a0..89f13ba 100644 --- a/js/src/bin/cli.js +++ b/js/src/bin/cli.js @@ -523,7 +523,10 @@ async function runWithIsolation( deferCommand, }) ); - console.log(''); + // Only print empty line when not deferring command (docker isolation handles its own spacing) + if (!deferCommand) { + console.log(''); + } // Save initial execution record and set global reference for signal cleanup if (executionRecord && store) { diff --git a/js/src/lib/docker-utils.js b/js/src/lib/docker-utils.js index 5768966..7e6c587 100644 --- a/js/src/lib/docker-utils.js +++ b/js/src/lib/docker-utils.js @@ -115,8 +115,9 @@ function dockerPullImage(image) { createTimelineSeparator, } = require('./output-blocks'); - // Print the virtual command line + // Print the virtual command line followed by empty line for visual separation console.log(createVirtualCommandBlock(`docker pull ${image}`)); + console.log(); let output = ''; let success = false; @@ -141,8 +142,7 @@ function dockerPullImage(image) { success = false; } - // Print result marker and separator - console.log(); + // Print result marker and separator (no empty line needed - already printed after command) console.log(createVirtualCommandResult(success)); console.log(createTimelineSeparator()); diff --git a/js/test/output-blocks.test.js b/js/test/output-blocks.test.js index fd799b2..f950c3d 100644 --- a/js/test/output-blocks.test.js +++ b/js/test/output-blocks.test.js @@ -458,6 +458,88 @@ describe('output-blocks module', () => { }); expect(block).not.toContain('$ echo hello'); }); + + it('should end with empty timeline line when deferCommand is true (issue #73)', () => { + // Issue #73: Visual continuity - when deferCommand is true, + // the start block should end with │ (empty timeline line) + // and no trailing empty line should be added by the CLI + const block = createStartBlock({ + sessionId: 'test-uuid', + timestamp: '2025-01-01 00:00:00', + command: 'echo hello', + extraLines: [ + '[Isolation] Environment: docker, Mode: attached', + '[Isolation] Image: alpine:latest', + '[Isolation] Session: docker-container-123', + ], + deferCommand: true, + }); + const lines = block.split('\n'); + // Last line should be empty timeline line (│) + expect(lines[lines.length - 1]).toBe('│'); + // Should not contain the command + expect(block).not.toContain('$ echo hello'); + }); + }); + + describe('Visual Continuity (issue #73)', () => { + it('virtual command block should be followed by empty line for visual separation', () => { + // Issue #73: The empty line should come AFTER the command + // ($ docker pull ...), not BEFORE it + // This test verifies the expected format: command line followed by + // empty line before output + const virtualCommand = createVirtualCommandBlock( + 'docker pull alpine:latest' + ); + expect(virtualCommand).toBe('$ docker pull alpine:latest'); + // The empty line is added by the caller (dockerPullImage) + // after the command and before running docker pull + }); + + it('timeline separator should be empty timeline line for visual structure', () => { + // After virtual command output, we print result marker and separator + const separator = createTimelineSeparator(); + expect(separator).toBe('│'); + }); + + it('start block with docker isolation should end cleanly for virtual commands', () => { + // When using docker isolation with defer_command, the block ends with │ + // The virtual command (docker pull) starts immediately after + const block = createStartBlock({ + sessionId: 'uuid-abc', + timestamp: '2026-01-08 12:00:00', + command: 'echo hi', + extraLines: [ + '[Isolation] Environment: docker, Mode: attached', + '[Isolation] Image: alpine:latest', + '[Isolation] Session: docker-1234', + ], + deferCommand: true, + }); + + // Expected structure: + // │ session uuid-abc + // │ start 2026-01-08 12:00:00 + // │ + // │ isolation docker + // │ mode attached + // │ image alpine:latest + // │ container docker-1234 + // │ <-- ends here with empty timeline line + const lines = block.split('\n'); + expect(lines[lines.length - 1]).toBe('│'); + + // Next should be virtual command (added separately by docker-utils): + // $ docker pull alpine:latest + // + // + // ✓ + // │ + // $ echo hi + // + // hi + // ✓ + }); }); }); diff --git a/rust/changelog.d/73.md b/rust/changelog.d/73.md index faf95d8..87bf489 100644 --- a/rust/changelog.d/73.md +++ b/rust/changelog.d/73.md @@ -1,8 +1,25 @@ -fix: Remove empty line after virtual command to maintain visual continuity +fix: Fix visual continuity in docker isolation mode -- Fixed visual continuity break in docker pull output -- Removed empty line between timeline marker (`│`) and virtual command line (`$ docker pull`) -- Output now flows continuously from timeline marker to command to output -- Applies to all virtual command blocks (currently docker pull operations) +- Fixed empty line placement in docker isolation output +- Empty line is now correctly placed AFTER the command (`$ docker pull alpine:latest`) + instead of BEFORE it +- Maintains consistent visual structure in timeline output +- Added tests for visual continuity behavior + +Before: +``` +│ + +$ docker pull alpine:latest +latest: Pulling from library/alpine +``` + +After: +``` +│ +$ docker pull alpine:latest + +latest: Pulling from library/alpine +``` Fixes #73 diff --git a/rust/src/bin/main.rs b/rust/src/bin/main.rs index 2acba54..1501d31 100644 --- a/rust/src/bin/main.rs +++ b/rust/src/bin/main.rs @@ -542,7 +542,10 @@ fn run_with_isolation( defer_command: is_docker_isolation, }) ); - println!(); + // Only print empty line when not deferring command (docker isolation handles its own spacing) + if !is_docker_isolation { + println!(); + } // Create log header let mut log_content = create_log_header(&LogHeaderParams { diff --git a/rust/src/lib/isolation.rs b/rust/src/lib/isolation.rs index 5fbb808..4c0118c 100644 --- a/rust/src/lib/isolation.rs +++ b/rust/src/lib/isolation.rs @@ -483,11 +483,12 @@ pub fn docker_image_exists(image: &str) -> bool { pub fn docker_pull_image(image: &str) -> (bool, String) { use std::io::{BufRead, BufReader}; - // Print the virtual command line + // Print the virtual command line followed by empty line for visual separation println!( "{}", crate::output_blocks::create_virtual_command_block(&format!("docker pull {}", image)) ); + println!(); let mut child = match Command::new("docker") .args(["pull", image]) @@ -532,8 +533,7 @@ pub fn docker_pull_image(image: &str) -> (bool, String) { let success = child.wait().map(|s| s.success()).unwrap_or(false); - // Print result marker and separator - println!(); + // Print result marker and separator (no empty line needed - already printed after command) println!( "{}", crate::output_blocks::create_virtual_command_result(success) diff --git a/rust/tests/output_blocks_test.rs b/rust/tests/output_blocks_test.rs index 86b75f5..f3eb4e5 100644 --- a/rust/tests/output_blocks_test.rs +++ b/rust/tests/output_blocks_test.rs @@ -273,3 +273,80 @@ fn test_escape_for_links_notation_with_double_quotes() { fn test_escape_for_links_notation_with_single_quotes() { assert_eq!(escape_for_links_notation("it's cool"), "\"it's cool\""); } + +// Issue #73: Visual Continuity Tests +#[test] +fn test_start_block_ends_with_empty_timeline_when_defer_command_true() { + // Issue #73: When deferCommand is true (docker isolation), + // the start block should end with │ (empty timeline line) + // and no trailing empty line should be added by the CLI + let extra = vec![ + "[Isolation] Environment: docker, Mode: attached", + "[Isolation] Image: alpine:latest", + "[Isolation] Session: docker-container-123", + ]; + let block = create_start_block(&StartBlockOptions { + session_id: "test-uuid", + timestamp: "2026-01-08 12:00:00", + command: "echo hello", + extra_lines: Some(extra), + style: None, + width: None, + defer_command: true, + }); + + let lines: Vec<&str> = block.lines().collect(); + // Last line should be empty timeline line (│) + assert_eq!( + lines[lines.len() - 1], + "│", + "Start block with defer_command should end with empty timeline line" + ); + // Should not contain the command + assert!( + !block.contains("$ echo hello"), + "Command should not appear when defer_command is true" + ); +} + +#[test] +fn test_visual_continuity_start_block_for_docker_isolation() { + // Issue #73: Visual continuity - the start block with docker isolation + // should end cleanly for virtual commands to follow immediately + let extra = vec![ + "[Isolation] Environment: docker, Mode: attached", + "[Isolation] Image: alpine:latest", + "[Isolation] Session: docker-1234", + ]; + let block = create_start_block(&StartBlockOptions { + session_id: "uuid-abc", + timestamp: "2026-01-08 12:00:00", + command: "echo hi", + extra_lines: Some(extra), + style: None, + width: None, + defer_command: true, + }); + + // Expected structure: + // │ session uuid-abc + // │ start 2026-01-08 12:00:00 + // │ + // │ isolation docker + // │ mode attached + // │ image alpine:latest + // │ container docker-1234 + // │ <-- ends here with empty timeline line + + let lines: Vec<&str> = block.lines().collect(); + assert_eq!( + lines[lines.len() - 1], + "│", + "Should end with empty timeline line" + ); + assert!( + block.contains("│ container docker-1234"), + "Should contain container name" + ); + assert!(!block.contains("$ echo hi"), "Command should be deferred"); +}