Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions js/.changeset/fix-visual-continuity.md
Original file line number Diff line number Diff line change
@@ -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
```
5 changes: 4 additions & 1 deletion js/src/bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions js/src/lib/docker-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());

Expand Down
82 changes: 82 additions & 0 deletions js/test/output-blocks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
// <empty line>
// <docker output>
// ✓
// │
// $ echo hi
// <empty line>
// hi
// ✓
});
});
});

Expand Down
27 changes: 22 additions & 5 deletions rust/changelog.d/73.md
Original file line number Diff line number Diff line change
@@ -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
5 changes: 4 additions & 1 deletion rust/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions rust/src/lib/isolation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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)
Expand Down
77 changes: 77 additions & 0 deletions rust/tests/output_blocks_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}