Skip to content

Commit aad3abe

Browse files
robobunClaude BotclaudeRiskyMHautofix-ci[bot]
authored
Update interactive spacing (#21156)
Co-authored-by: Claude Bot <[email protected]> Co-authored-by: Claude <[email protected]> Co-authored-by: RiskyMH <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jarred Sumner <[email protected]>
1 parent 6d5637b commit aad3abe

File tree

4 files changed

+214
-56
lines changed

4 files changed

+214
-56
lines changed

src/cli/update_interactive_command.zig

Lines changed: 49 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,13 @@ pub const UpdateInteractiveCommand = struct {
562562
return result;
563563
}
564564

565+
const ColumnWidths = struct {
566+
name: usize,
567+
current: usize,
568+
target: usize,
569+
latest: usize,
570+
};
571+
565572
const MultiSelectState = struct {
566573
packages: []OutdatedPackage,
567574
selected: []bool,
@@ -573,26 +580,13 @@ pub const UpdateInteractiveCommand = struct {
573580
max_latest_len: usize = 0,
574581
};
575582

576-
fn promptForUpdates(allocator: std.mem.Allocator, packages: []OutdatedPackage) ![]bool {
577-
if (packages.len == 0) {
578-
Output.prettyln("<r><green>✓<r> All packages are up to date!", .{});
579-
return allocator.alloc(bool, 0);
580-
}
581-
582-
const selected = try allocator.alloc(bool, packages.len);
583-
// Default to all unselected
584-
@memset(selected, false);
585-
586-
// Calculate max column widths - need to account for diffFmt ANSI codes
583+
fn calculateColumnWidths(packages: []OutdatedPackage) ColumnWidths {
584+
// Calculate natural widths based on content
587585
var max_name_len: usize = "Package".len;
588586
var max_current_len: usize = "Current".len;
589587
var max_target_len: usize = "Target".len;
590588
var max_latest_len: usize = "Latest".len;
591589

592-
// Set reasonable limits to prevent excessive column widths
593-
const MAX_NAME_WIDTH = 60;
594-
const MAX_VERSION_WIDTH = 20;
595-
596590
for (packages) |pkg| {
597591
// Include dev tag length in max calculation
598592
var dev_tag_len: usize = 0;
@@ -603,26 +597,42 @@ pub const UpdateInteractiveCommand = struct {
603597
} else if (pkg.behavior.optional) {
604598
dev_tag_len = 9; // " optional"
605599
}
606-
const name_len = @min(pkg.name.len + dev_tag_len, MAX_NAME_WIDTH);
607-
max_name_len = @max(max_name_len, name_len);
608600

609-
// For current version - it's always displayed without formatting
610-
max_current_len = @max(max_current_len, @min(pkg.current_version.len, MAX_VERSION_WIDTH));
601+
max_name_len = @max(max_name_len, pkg.name.len + dev_tag_len);
602+
max_current_len = @max(max_current_len, pkg.current_version.len);
603+
max_target_len = @max(max_target_len, pkg.update_version.len);
604+
max_latest_len = @max(max_latest_len, pkg.latest_version.len);
605+
}
606+
607+
// Use natural widths without any limits
608+
return ColumnWidths{
609+
.name = max_name_len,
610+
.current = max_current_len,
611+
.target = max_target_len,
612+
.latest = max_latest_len,
613+
};
614+
}
611615

612-
// For max width calculation, just use the plain version string length
613-
// The diffFmt adds ANSI codes but doesn't change the visible width of the version
614-
// Version strings are always ASCII so we can use string length directly
615-
max_target_len = @max(max_target_len, @min(pkg.update_version.len, MAX_VERSION_WIDTH));
616-
max_latest_len = @max(max_latest_len, @min(pkg.latest_version.len, MAX_VERSION_WIDTH));
616+
fn promptForUpdates(allocator: std.mem.Allocator, packages: []OutdatedPackage) ![]bool {
617+
if (packages.len == 0) {
618+
Output.prettyln("<r><green>✓<r> All packages are up to date!", .{});
619+
return allocator.alloc(bool, 0);
617620
}
618621

622+
const selected = try allocator.alloc(bool, packages.len);
623+
// Default to all unselected
624+
@memset(selected, false);
625+
626+
// Calculate optimal column widths based on terminal width and content
627+
const columns = calculateColumnWidths(packages);
628+
619629
var state = MultiSelectState{
620630
.packages = packages,
621631
.selected = selected,
622-
.max_name_len = max_name_len,
623-
.max_current_len = max_current_len,
624-
.max_update_len = max_target_len,
625-
.max_latest_len = max_latest_len,
632+
.max_name_len = columns.name,
633+
.max_current_len = columns.current,
634+
.max_update_len = columns.target,
635+
.max_latest_len = columns.latest,
626636
};
627637

628638
// Set raw mode
@@ -745,10 +755,11 @@ pub const UpdateInteractiveCommand = struct {
745755
// The padding should align with the first character of package names
746756
// Package names start at: " " (4 spaces) + "□ " (2 chars) = 6 chars from left
747757
// Headers start at: " " (2 spaces) + dep_type_text
748-
// So we need to pad: 4 (cursor/space) + 2 (checkbox+space) + max_name_len - dep_type_text_len
749-
// Use safe subtraction to prevent underflow when dep_type_text_len is longer than available space
750-
const base_padding = 4 + 2 + state.max_name_len;
751-
const padding_to_current = if (dep_type_text_len >= base_padding) 1 else base_padding - dep_type_text_len;
758+
// We need the headers to align where the current version column starts
759+
// That's at: 6 (start of names) + max_name_len + 2 (spacing after names) - 2 (header indent) - dep_type_text_len
760+
const total_offset = 6 + state.max_name_len + 2;
761+
const header_start = 2 + dep_type_text_len;
762+
const padding_to_current = if (header_start >= total_offset) 1 else total_offset - header_start;
752763
while (j < padding_to_current) : (j += 1) {
753764
Output.print(" ", .{});
754765
}
@@ -869,14 +880,7 @@ pub const UpdateInteractiveCommand = struct {
869880
"";
870881
defer if (package_url.len > 0) bun.default_allocator.free(package_url);
871882

872-
// Truncate package name if it's too long
873-
const display_name = if (pkg.name.len > 60)
874-
try std.fmt.allocPrint(bun.default_allocator, "{s}...", .{pkg.name[0..57]})
875-
else
876-
pkg.name;
877-
defer if (display_name.ptr != pkg.name.ptr) bun.default_allocator.free(display_name);
878-
879-
const hyperlink = TerminalHyperlink.new(package_url, display_name, package_url.len > 0);
883+
const hyperlink = TerminalHyperlink.new(package_url, pkg.name, package_url.len > 0);
880884

881885
if (selected) {
882886
if (strings.eqlComptime(checkbox_color, "red")) {
@@ -899,24 +903,17 @@ pub const UpdateInteractiveCommand = struct {
899903
Output.pretty("<r><d> optional<r>", .{});
900904
}
901905

902-
// Print padding after name
906+
// Print padding after name (2 spaces)
903907
var j: usize = 0;
904908
while (j < name_padding + 2) : (j += 1) {
905909
Output.print(" ", .{});
906910
}
907911

908-
// Current version - truncate if too long
909-
const display_current = if (pkg.current_version.len > 20)
910-
try std.fmt.allocPrint(bun.default_allocator, "{s}...", .{pkg.current_version[0..17]})
911-
else
912-
pkg.current_version;
913-
defer if (display_current.ptr != pkg.current_version.ptr) bun.default_allocator.free(display_current);
914-
915-
Output.pretty("<r>{s}<r>", .{display_current});
912+
// Current version
913+
Output.pretty("<r>{s}<r>", .{pkg.current_version});
916914

917-
// Print padding after current version
918-
const current_display_len = if (pkg.current_version.len > 20) 20 else pkg.current_version.len;
919-
const current_padding = if (current_display_len >= state.max_current_len) 0 else state.max_current_len - current_display_len;
915+
// Print padding after current version (2 spaces)
916+
const current_padding = if (pkg.current_version.len >= state.max_current_len) 0 else state.max_current_len - pkg.current_version.len;
920917
j = 0;
921918
while (j < current_padding + 2) : (j += 1) {
922919
Output.print(" ", .{});

src/output.zig

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ var stdout_stream: Source.StreamType = undefined;
2222
var stdout_stream_set = false;
2323
const File = bun.sys.File;
2424
pub var terminal_size: std.posix.winsize = .{
25-
.ws_row = 0,
26-
.ws_col = 0,
27-
.ws_xpixel = 0,
28-
.ws_ypixel = 0,
25+
.row = 0,
26+
.col = 0,
27+
.xpixel = 0,
28+
.ypixel = 0,
2929
};
3030

3131
pub const Source = struct {
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Bun Snapshot v1, https://bun.sh/docs/test/snapshots
2+
3+
exports[`bun update --interactive snapshots should format mixed package types with proper spacing: update-interactive-formatting 1`] = `"bun update --interactive vX.X.X"`;
4+
5+
exports[`bun update --interactive snapshots should not crash with various package name lengths: update-interactive-no-crash 1`] = `"bun update --interactive vX.X.X"`;
6+
7+
exports[`bun update --interactive snapshots should handle extremely long package names without crashing: update-interactive-long-names 1`] = `"bun update --interactive vX.X.X"`;
8+
9+
exports[`bun update --interactive snapshots should handle complex version strings without crashing: update-interactive-complex-versions 1`] = `"bun update --interactive vX.X.X"`;
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
import { describe, expect, it } from "bun:test";
2+
import { bunEnv, bunExe, tempDirWithFiles } from "harness";
3+
4+
describe("bun update --interactive snapshots", () => {
5+
it("should not crash with various package name lengths", async () => {
6+
const dir = tempDirWithFiles("update-interactive-snapshot-test", {
7+
"package.json": JSON.stringify({
8+
name: "test-project",
9+
version: "1.0.0",
10+
dependencies: {
11+
"short": "1.0.0",
12+
"react": "17.0.2",
13+
"really-long-package-name-for-testing": "1.0.0",
14+
"@scoped/package": "1.0.0",
15+
"@organization/extremely-long-scoped-package-name": "1.0.0",
16+
},
17+
devDependencies: {
18+
"dev-pkg": "1.0.0",
19+
"super-long-dev-package-name-for-testing": "1.0.0",
20+
"typescript": "4.8.0",
21+
},
22+
peerDependencies: {
23+
"peer-pkg": "1.0.0",
24+
"very-long-peer-dependency-name": "1.0.0",
25+
},
26+
optionalDependencies: {
27+
"optional-pkg": "1.0.0",
28+
"long-optional-dependency-name": "1.0.0",
29+
},
30+
}),
31+
});
32+
33+
// Test that the command doesn't crash with mixed package lengths
34+
const result = await Bun.spawn({
35+
cmd: [bunExe(), "update", "--interactive", "--dry-run"],
36+
cwd: dir,
37+
env: bunEnv,
38+
stdin: "pipe",
39+
stdout: "pipe",
40+
stderr: "pipe",
41+
});
42+
43+
// Send 'n' to exit without selecting anything
44+
result.stdin.write("n\n");
45+
result.stdin.end();
46+
47+
const stdout = await new Response(result.stdout).text();
48+
const stderr = await new Response(result.stderr).text();
49+
50+
// Replace version numbers and paths to avoid flakiness
51+
const normalizedOutput = normalizeOutput(stdout);
52+
53+
// The output should show proper column spacing and formatting
54+
expect(normalizedOutput).toMatchSnapshot("update-interactive-no-crash");
55+
56+
// Should not crash or have formatting errors
57+
expect(stderr).not.toContain("panic");
58+
expect(stderr).not.toContain("underflow");
59+
expect(stderr).not.toContain("overflow");
60+
});
61+
62+
it("should handle extremely long package names without crashing", async () => {
63+
const veryLongName = "a".repeat(80);
64+
const dir = tempDirWithFiles("update-interactive-long-names", {
65+
"package.json": JSON.stringify({
66+
name: "test-project",
67+
version: "1.0.0",
68+
dependencies: {
69+
[veryLongName]: "1.0.0",
70+
"regular-package": "1.0.0",
71+
},
72+
}),
73+
});
74+
75+
const result = await Bun.spawn({
76+
cmd: [bunExe(), "update", "--interactive", "--dry-run"],
77+
cwd: dir,
78+
env: bunEnv,
79+
stdin: "pipe",
80+
stdout: "pipe",
81+
stderr: "pipe",
82+
});
83+
84+
result.stdin.write("n\n");
85+
result.stdin.end();
86+
87+
const stdout = await new Response(result.stdout).text();
88+
const stderr = await new Response(result.stderr).text();
89+
90+
const normalizedOutput = normalizeOutput(stdout);
91+
92+
// Should not crash
93+
expect(normalizedOutput).toMatchSnapshot("update-interactive-long-names");
94+
expect(stderr).not.toContain("panic");
95+
expect(stderr).not.toContain("underflow");
96+
});
97+
98+
it("should handle complex version strings without crashing", async () => {
99+
const dir = tempDirWithFiles("update-interactive-complex-versions", {
100+
"package.json": JSON.stringify({
101+
name: "test-project",
102+
version: "1.0.0",
103+
dependencies: {
104+
"package-with-long-version": "1.0.0-alpha.1.2.3.4.5.6.7.8.9.10.11.12",
105+
"package-with-prerelease": "1.0.0-beta.1+build.12345",
106+
"package-with-short-version": "1.0.0",
107+
},
108+
}),
109+
});
110+
111+
const result = await Bun.spawn({
112+
cmd: [bunExe(), "update", "--interactive", "--dry-run"],
113+
cwd: dir,
114+
env: bunEnv,
115+
stdin: "pipe",
116+
stdout: "pipe",
117+
stderr: "pipe",
118+
});
119+
120+
result.stdin.write("n\n");
121+
result.stdin.end();
122+
123+
const stdout = await new Response(result.stdout).text();
124+
const stderr = await new Response(result.stderr).text();
125+
126+
const normalizedOutput = normalizeOutput(stdout);
127+
128+
// Should not crash
129+
expect(normalizedOutput).toMatchSnapshot("update-interactive-complex-versions");
130+
expect(stderr).not.toContain("panic");
131+
expect(stderr).not.toContain("underflow");
132+
});
133+
});
134+
135+
function normalizeOutput(output: string): string {
136+
// Remove Bun version to avoid test flakiness
137+
let normalized = output.replace(/bun update --interactive v\d+\.\d+\.\d+[^\n]*/g, "bun update --interactive vX.X.X");
138+
139+
// Normalize any absolute paths
140+
normalized = normalized.replace(/\/tmp\/[^\/\s]+/g, "/tmp/test-dir");
141+
142+
// Remove ANSI color codes for cleaner snapshots
143+
normalized = normalized.replace(/\x1b\[[0-9;]*m/g, "");
144+
145+
// Remove progress indicators and timing info
146+
normalized = normalized.replace(/[\r\n]*\s*\([0-9.]+ms\)/g, "");
147+
148+
// Normalize whitespace
149+
normalized = normalized.replace(/\r\n/g, "\n");
150+
151+
return normalized.trim();
152+
}

0 commit comments

Comments
 (0)