From e31a74e829685583105a798e2e575b01cdf05526 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Fri, 9 Oct 2026 11:51:25 -0700 Subject: [PATCH] remove duplication --- .kiro/specs/calculator/tasks.md | 50 +++++ src/cli/format.zig | 320 ++++++++++++++++---------------- src/main.zig | 296 ++++++++++++++--------------- 3 files changed, 356 insertions(+), 310 deletions(-) diff --git a/.kiro/specs/calculator/tasks.md b/.kiro/specs/calculator/tasks.md index d386b48..bcb774a 100644 --- a/.kiro/specs/calculator/tasks.md +++ b/.kiro/specs/calculator/tasks.md @@ -950,6 +950,56 @@ plans a `--raw` flag, but today it is exercised only by tests. 100% line coverage, engine 99.44%. CLI output byte-identical across all five rows, both byte orders, ASCII packing and the multi-base standard-mode view. +### Task 5.30: One output path in the CLI, and redirected output that was overwriting itself + +A review of `format.zig` and `main.zig` after the struct work, for duplication and for +`pub` that nothing outside the file needs. It found one real defect along the way. + +**Redirected output was corrupt.** `main`'s `write` made a fresh `File.writer` per call, +and that writer is positional: on a regular file it writes from offset 0. Every +single-line result was written and then followed by a separate `"\n"`, which landed on +the result's first byte: + +``` +$ tally convert 100 km mi > out; od -c out +\n 0 0 k m = 6 2 . 1 3 ... was +1 0 0 k m = 6 2 . 1 3 ... m i \n now +``` + +Pipes and terminals are not seekable, which is why nothing noticed: every check of the +output had gone through one. `write` now uses `writerStreaming`, and output is written +once. + +**One newline rule.** Errors and multi-line results carried their own final newline; +single-line results relied on `main` to add one; programmer mode did both, so `tally -p 1` +ended in a blank line. A `format.Result` is now complete text ending in exactly one +newline, `main` prints it as it stands, and a test holds every kind of result, success +and error, to that. Removing the newline from one path fails it. + +**One output path.** `main` had five copies of "pick the stream, write, maybe add a +newline, return the status"; it now builds one `Result` and writes it once. +`ParsedArgs.output` was the same shape as `format.Result` under another name and is now +`done: format.Result`, built through `shown` (help, stdout), `refused` (a usage error) +and `failed` (an engine error), which replace 26 hand-built returns. `readStructSource` +returns its failure as a `Result` rather than printing it, so nothing else writes. The +TUI's failed-start message went from three writes to one. + +**Wording that belongs to the engine.** Nine `"error: out of memory\n"` and six +`"error: invalid number\n"` literals retyped what `engine.phrase` already says; they are +`Result.fail(err)`. "buffer overflow" (eight places, and it reads like a security bug) and +"result too long to display" (one) were one condition with two messages, and are now +`Result.too_long`. `amortization` writes through one allocating writer instead of a line +buffer and a copy per line, with an amount too large to render still told apart from +running out of memory. + +**`pub` that nothing needed:** `ParsedArgs` and `parseArgs` (nothing imports +`main.zig`), and `decoratedError` once `Result.fail` became its only caller. + +- Verify: 1047 tests pass in Debug and every release mode, fmt and zlint clean. Every + subcommand's stdout, stderr and exit status compared against the previous build + through a pipe across 34 invocations: identical except `-p`, which lost its blank line. + `main.zig` 100% line coverage, `format.zig` 99.4%. + ### Task 5.29: One word, and the name it was hiding in Entry 17 covered `build.zig`, `build/Coverage.zig` and `build/download_kcov.zig`, and diff --git a/src/cli/format.zig b/src/cli/format.zig index a70ef71..60d1d4a 100644 --- a/src/cli/format.zig +++ b/src/cli/format.zig @@ -17,11 +17,31 @@ const std = @import("std"); const engine = @import("engine"); -/// A finished piece of output: the text, and whether it is an error. `main.zig` turns -/// the flag into a stream choice and an exit status. +/// A finished piece of output: complete text, ending in a newline, and whether it is +/// an error. `main.zig` turns the flag into a stream and an exit status and prints the +/// text as it stands. +/// +/// The newline is part of the text because some outputs are several lines and some +/// are one. It used to be split: errors and multi-line results carried their own, +/// single-line results relied on `main` to add one, and programmer mode did both, so +/// `tally -p 1` ended in a blank line. pub const Result = struct { output: []const u8, is_error: bool, + + pub fn ok(output: []const u8) Result { + return .{ .output = output, .is_error = false }; + } + + /// An engine error, in the engine's words. + pub fn fail(err: engine.Error) Result { + return .{ .output = decoratedError(err), .is_error = true }; + } + + /// Text that does not fit the buffer it was written into. One message for one + /// condition: there used to be "buffer overflow" in eight places and "result too + /// long to display" in a ninth, and the first reads like a security bug. + const too_long: Result = .{ .output = "error: the result is too long to display\n", .is_error = true }; }; /// Which evaluator a CLI invocation wants. @@ -67,7 +87,7 @@ pub fn evaluateWith( ) Result { if (mode == .programmer) { const result = engine.evalProgrammerString(allocator, expression, config) catch |err| { - return .{ .output = decoratedError(err), .is_error = true }; + return .fail(err); }; return programmerResult(buf, result, config); } @@ -79,42 +99,27 @@ pub fn evaluateWith( // "32F to C". Anything without it falls through to normal evaluation. if (engine.units.parseRequest(expression)) |maybe_request| { if (maybe_request) |request| { - var value = engine.evalString(&env, allocator, request.value_text) catch |err| { - return .{ .output = decoratedError(err), .is_error = true }; - }; + var value = engine.evalString(&env, allocator, request.value_text) catch |err| return .fail(err); defer value.deinit(); return conversionUnits(allocator, buf, value, request.from, request.to); } - } else |err| { - return .{ .output = decoratedError(err), .is_error = true }; - } + } else |err| return .fail(err); - var info = engine.evalStringInfo(&env, allocator, expression) catch |err| { - return .{ .output = decoratedError(err), .is_error = true }; - }; + var info = engine.evalStringInfo(&env, allocator, expression) catch |err| return .fail(err); defer info.value.deinit(); + const shown = info.value.render(allocator, display_format) catch |err| return .fail(err); + defer shown.deinit(allocator); + // Enrich with multi-base view when the expression used non-decimal // literals and the result is a non-negative integer. if (info.has_nondecimal_literal and isDisplayableInt(info.value.toFloat(allocator))) { - const decimal = info.value.render(allocator, display_format) catch { - return .{ .output = "error: out of memory\n", .is_error = true }; - }; - defer decimal.deinit(allocator); - return standardMultiBase(buf, decimal.text, info.value.toFloat(allocator)); + return standardMultiBase(buf, shown.text, info.value.toFloat(allocator)); } - const shown = info.value.render(allocator, display_format) catch { - return .{ .output = "error: out of memory\n", .is_error = true }; - }; - defer shown.deinit(allocator); - // Copy into the caller's buffer so the result does not depend on the - // allocator outliving this call. - if (shown.text.len > buf.len) { - return .{ .output = "error: result too long to display\n", .is_error = true }; - } - @memcpy(buf[0..shown.text.len], shown.text); - return .{ .output = buf[0..shown.text.len], .is_error = false }; + // Into the caller's buffer, so the result does not depend on the allocator + // outliving this call. + return .ok(std.fmt.bufPrint(buf, "{s}\n", .{shown.text}) catch return .too_long); } /// True if the f64 is a non-negative integer within u128 range. @@ -135,22 +140,16 @@ fn standardMultiBase(buf: []u8, dec_display: []const u8, value: f64) Result { // The rows print themselves, so nothing here needs a buffer per base. // `dec_display` is the caller's allocated text rather than a slice of `buf`, // so writing into `buf` cannot clobber it and it needs no copy first. - var w = std.Io.Writer.fixed(buf); - w.print("{s}\n hex: {f}\n oct: {f}\n bin: {f}", .{ + return .ok(std.fmt.bufPrint(buf, "{s}\n hex: {f}\n oct: {f}\n bin: {f}\n", .{ dec_display, int.fmt(.hex, .{}), int.fmt(.octal, .{}), int.fmt(.binary, .{}), - }) catch { - return .{ .output = "error: buffer overflow\n", .is_error = true }; - }; - - return .{ .output = w.buffered(), .is_error = false }; + }) catch return .too_long); } fn programmerResult(buf: []u8, result: engine.Integer, config: engine.programmer.Config) Result { - var w = std.Io.Writer.fixed(buf); - w.print( + return .ok(std.fmt.bufPrint(buf, \\ dec(signed): {f} \\ dec(unsigned): {f} \\ hex: {f} @@ -163,11 +162,7 @@ fn programmerResult(buf: []u8, result: engine.Integer, config: engine.programmer result.fmt(.hex, .{ .endian = config.display_endian }), result.fmt(.octal, .{}), result.fmt(.binary, .{}), - }) catch { - return .{ .output = "error: buffer overflow\n", .is_error = true }; - }; - - return .{ .output = w.buffered(), .is_error = false }; + }) catch return .too_long); } // -- Conversions -- @@ -187,9 +182,7 @@ pub fn conversion( ) Result { // Parse the value exactly rather than through f64, so a decimal input like // 2.5 enters the conversion without being rounded first. - var value = engine.Number.parse(allocator, value_text) catch { - return .{ .output = "error: invalid number\n", .is_error = true }; - }; + var value = engine.Number.parse(allocator, value_text) catch |err| return .fail(err); defer value.deinit(); return conversionUnits(allocator, buf, value, from, to); @@ -207,26 +200,17 @@ fn conversionUnits( from: engine.UnitDef, to: engine.UnitDef, ) Result { - var converted = engine.units.convertExactUnits(allocator, value, from, to) catch |err| { - return .{ .output = decoratedError(err), .is_error = true }; - }; + var converted = engine.units.convertExactUnits(allocator, value, from, to) catch |err| return .fail(err); defer converted.deinit(); - const shown_in = value.render(allocator, display_format) catch { - return .{ .output = "error: out of memory\n", .is_error = true }; - }; + const shown_in = value.render(allocator, display_format) catch |err| return .fail(err); defer shown_in.deinit(allocator); - const shown_out = converted.render(allocator, display_format) catch { - return .{ .output = "error: out of memory\n", .is_error = true }; - }; + const shown_out = converted.render(allocator, display_format) catch |err| return .fail(err); defer shown_out.deinit(allocator); - const output = std.fmt.bufPrint(buf, "{s} {s} = {s} {s}", .{ + return .ok(std.fmt.bufPrint(buf, "{s} {s} = {s} {s}\n", .{ shown_in.text, from.name, shown_out.text, to.name, - }) catch { - return .{ .output = "error: buffer overflow\n", .is_error = true }; - }; - return .{ .output = output, .is_error = false }; + }) catch return .too_long); } // -- Amortization -- @@ -238,75 +222,65 @@ pub fn amortization( params: engine.financial.AmortizationParams, summary_only: bool, ) Result { - const payment = engine.financial.amortizationPayment(params) catch |err| { - return .{ .output = amortizationError(err), .is_error = true }; - }; - const rows = engine.financial.amortizationSchedule(allocator, params) catch |err| { - return .{ .output = amortizationError(err), .is_error = true }; - }; + const payment = engine.financial.amortizationPayment(params) catch |err| return amortizationError(err); + const totals = engine.financial.amortizationTotals(params) catch |err| return amortizationError(err); + const rows = engine.financial.amortizationSchedule(allocator, params) catch |err| return amortizationError(err); defer allocator.free(rows); - var out = std.ArrayList(u8).empty; - errdefer out.deinit(allocator); - var line: [256]u8 = undefined; - var money_buf: [64]u8 = undefined; + var out: std.Io.Writer.Allocating = .init(allocator); + defer out.deinit(); + writeSchedule(&out.writer, params, payment, rows, totals, summary_only) catch |err| return switch (err) { + error.Unformattable => unformattable, + error.WriteFailed => .fail(error.OutOfMemory), + }; + return .ok(out.toOwnedSlice() catch return .fail(error.OutOfMemory)); +} - const header = std.fmt.bufPrint(&line, "{s} at {d}% per period over {d} periods\n", .{ - money(params.principal).render(&money_buf) catch return unformattable(), - params.rate, - params.periods, - }) catch return .{ .output = "error: buffer overflow\n", .is_error = true }; - out.appendSlice(allocator, header) catch return oom(); - - const payment_line = std.fmt.bufPrint(&line, "Payment {s} per period\n\n", .{ - money(payment).render(&money_buf) catch return unformattable(), - }) catch return .{ .output = "error: buffer overflow\n", .is_error = true }; - out.appendSlice(allocator, payment_line) catch return oom(); +fn writeSchedule( + w: *std.Io.Writer, + params: engine.financial.AmortizationParams, + payment: f64, + rows: []const engine.financial.AmortizationEntry, + totals: engine.financial.AmortizationTotals, + summary_only: bool, +) (std.Io.Writer.Error || error{Unformattable})!void { + var a: [4]Amount = undefined; + try w.print("{s} at {d}% per period over {d} periods\n", .{ try a[0].of(params.principal), params.rate, params.periods }); + try w.print("Payment {s} per period\n\n", .{try a[0].of(payment)}); if (!summary_only) { - out.appendSlice( - allocator, - "Period Payment Interest Principal Balance\n", - ) catch return oom(); + try w.writeAll("Period Payment Interest Principal Balance\n"); for (rows) |row| { - var pay_buf: [64]u8 = undefined; - var int_buf: [64]u8 = undefined; - var prin_buf: [64]u8 = undefined; - var bal_buf: [64]u8 = undefined; - const row_text = std.fmt.bufPrint(&line, "{d: >6} {s: >12} {s: >12} {s: >12} {s: >12}\n", .{ + try w.print("{d: >6} {s: >12} {s: >12} {s: >12} {s: >12}\n", .{ row.period, - money(row.payment).render(&pay_buf) catch return unformattable(), - money(row.interest).render(&int_buf) catch return unformattable(), - money(row.principal).render(&prin_buf) catch return unformattable(), - money(row.balance).render(&bal_buf) catch return unformattable(), - }) catch return .{ .output = "error: buffer overflow\n", .is_error = true }; - out.appendSlice(allocator, row_text) catch return oom(); + try a[0].of(row.payment), + try a[1].of(row.interest), + try a[2].of(row.principal), + try a[3].of(row.balance), + }); } - out.appendSlice(allocator, "\n") catch return oom(); + try w.writeAll("\n"); } - const totals = engine.financial.amortizationTotals(params) catch |err| { - return .{ .output = amortizationError(err), .is_error = true }; - }; - var paid_buf: [64]u8 = undefined; - var interest_buf: [64]u8 = undefined; - var principal_buf: [64]u8 = undefined; - const summary = std.fmt.bufPrint( - &line, - "Periods paid {d}\nTotal paid {s}\nTotal interest {s}\nPrincipal {s}", - .{ - totals.periods, - money(totals.paid).render(&paid_buf) catch return unformattable(), - money(totals.interest).render(&interest_buf) catch return unformattable(), - money(totals.principal).render(&principal_buf) catch return unformattable(), - }, - ) catch return .{ .output = "error: buffer overflow\n", .is_error = true }; - out.appendSlice(allocator, summary) catch return oom(); - - const text = out.toOwnedSlice(allocator) catch return oom(); - return .{ .output = text, .is_error = false }; + try w.print("Periods paid {d}\nTotal paid {s}\nTotal interest {s}\nPrincipal {s}\n", .{ + totals.periods, + try a[0].of(totals.paid), + try a[1].of(totals.interest), + try a[2].of(totals.principal), + }); } +/// Room for one amount's text, for the columns that pad it. Rendered rather than +/// printed with `{f}` so that an amount too large to render is told apart from a +/// writer that has run out of memory. +const Amount = struct { + buf: [64]u8, + + fn of(self: *Amount, value: f64) error{Unformattable}![]const u8 { + return money(value).render(&self.buf) catch error.Unformattable; + } +}; + // -- Struct layout -- const struct_layout = engine.struct_layout; @@ -386,12 +360,12 @@ pub fn structLayout( var scratch: std.heap.ArenaAllocator = .init(allocator); defer scratch.deinit(); - const rows = tableRows(scratch.allocator(), definition, &layout) catch return oom(); + const rows = tableRows(scratch.allocator(), definition, &layout) catch |err| return .fail(err); var out: std.Io.Writer.Allocating = .init(allocator); defer out.deinit(); - writeLayout(&out.writer, rows, definition, &layout, paint) catch return oom(); - const text = out.toOwnedSlice() catch return oom(); + writeLayout(&out.writer, rows, definition, &layout, paint) catch return .fail(error.OutOfMemory); + const text = out.toOwnedSlice() catch |err| return .fail(err); return .{ .output = text, .is_error = false }; } @@ -400,8 +374,10 @@ pub fn structLayout( /// error: unknown type (typedefs and macros from elsewhere are not visible) /// line 3, column 5: uint33_t fn located(allocator: std.mem.Allocator, err: engine.Error, source: []const u8, span: ?struct_layout.Span) Result { - if (err == error.OutOfMemory) return oom(); - const s = span orelse return .{ .output = decoratedError(err), .is_error = true }; + // Running out of memory is about the machine, not a place in the input, and the + // parser's diagnostic always holds a span, so this comes first. + if (err == error.OutOfMemory) return .fail(err); + const s = span orelse return .fail(err); const at = CStructParser.locate(source, s.start); const text = source[s.start..][0..s.len]; // One line of it, and not much of that: a directive is reported whole. @@ -413,7 +389,7 @@ fn located(allocator: std.mem.Allocator, err: engine.Error, source: []const u8, at.column, if (shown.len == 0) ", at the end of the input" else ": ", shown, - }) catch return oom(); + }) catch return .fail(error.OutOfMemory); return .{ .output = output, .is_error = true }; } @@ -694,8 +670,8 @@ fn writeMap(w: *std.Io.Writer, layout: *const struct_layout.Layout, paint: Paint /// /// `inline else` makes this exhaustive with no fallback branch: an error added /// anywhere in the engine fails to compile in `engine.phrase` rather than silently -/// reading "evaluation error" here. -pub fn decoratedError(err: engine.Error) []const u8 { +/// reading "evaluation error" here. Reached through `Result.fail`. +fn decoratedError(err: engine.Error) []const u8 { return switch (err) { inline else => |e| comptime "error: " ++ engine.phrase(e) ++ "\n", }; @@ -703,29 +679,25 @@ pub fn decoratedError(err: engine.Error) []const u8 { /// The loan terms a schedule cannot be built from, in the words of the thing the /// user actually typed. -fn amortizationError(err: engine.Error) []const u8 { +fn amortizationError(err: engine.Error) Result { return switch (err) { - engine.Error.DomainError => // The realistic causes are all one of these, and a bare "domain error" // would leave the user guessing which. - "error: check the loan terms: principal and periods must be positive, the rate cannot be negative, and the payment must at least cover the first period's interest\n", - else => decoratedError(err), + engine.Error.DomainError => .{ + .output = "error: check the loan terms: principal and periods must be positive, the rate cannot be negative, and the payment must at least cover the first period's interest\n", + .is_error = true, + }, + else => .fail(err), }; } /// An amount too large to render. Reported rather than printed as a placeholder: /// the previous local formatter returned "?" for these, so a table of question /// marks came out with exit status 0. -fn unformattable() Result { - return .{ - .output = "error: an amount in this schedule is too large to format\n", - .is_error = true, - }; -} - -fn oom() Result { - return .{ .output = "error: out of memory\n", .is_error = true }; -} +const unformattable: Result = .{ + .output = "error: an amount in this schedule is too large to format\n", + .is_error = true, +}; // -- Tests -- @@ -744,7 +716,7 @@ test "evaluate: standard arithmetic" { var buf: [4096]u8 = undefined; const result = evaluateWith(arena.allocator(), "2 + 2", .standard, .{}, &buf); try testing.expect(!result.is_error); - try testing.expectEqualStrings("4", result.output); + try testing.expectEqualStrings("4\n", result.output); } test "evaluate: large number has commas" { @@ -753,7 +725,7 @@ test "evaluate: large number has commas" { var buf: [4096]u8 = undefined; const result = evaluateWith(arena.allocator(), "2^32 - 1", .standard, .{}, &buf); try testing.expect(!result.is_error); - try testing.expectEqualStrings("4,294,967,295", result.output); + try testing.expectEqualStrings("4,294,967,295\n", result.output); } test "evaluate: function" { @@ -762,7 +734,7 @@ test "evaluate: function" { var buf: [4096]u8 = undefined; const result = evaluateWith(arena.allocator(), "sin(pi/2) + 1", .standard, .{}, &buf); try testing.expect(!result.is_error); - try testing.expectEqualStrings("2", result.output); + try testing.expectEqualStrings("2\n", result.output); } test "evaluate: explicit mul" { @@ -771,7 +743,7 @@ test "evaluate: explicit mul" { var buf: [4096]u8 = undefined; const result = evaluateWith(arena.allocator(), "3*(4+5)", .standard, .{}, &buf); try testing.expect(!result.is_error); - try testing.expectEqualStrings("27", result.output); + try testing.expectEqualStrings("27\n", result.output); } test "evaluate: programmer mode" { @@ -816,6 +788,7 @@ test "evaluate: an expression written in another base answers in every base" { \\ hex: 01 00 \\ oct: 000 400 \\ bin: 0000 0001 0000 0000 + \\ , hex.output); // The rows are as wide as the value needs and no wider: 10 fits one byte, where @@ -828,6 +801,7 @@ test "evaluate: an expression written in another base answers in every base" { \\ hex: 0A \\ oct: 012 \\ bin: 0000 1010 + \\ , binary.output); // The decimal line is the ordinary rendering, so it still groups its digits. @@ -848,11 +822,11 @@ test "evaluate: a non-decimal literal with no whole answer stays decimal" { // print. const negative = evaluateWith(arena.allocator(), "0xFF - 0x100", .standard, .{}, &buf); try testing.expect(!negative.is_error); - try testing.expectEqualStrings("-1", negative.output); + try testing.expectEqualStrings("-1\n", negative.output); const fractional = evaluateWith(arena.allocator(), "0xFF / 2", .standard, .{}, &buf); try testing.expect(!fractional.is_error); - try testing.expectEqualStrings("127.5", fractional.output); + try testing.expectEqualStrings("127.5\n", fractional.output); } test "evaluate: multi-base rows that outgrow the buffer are refused, not truncated" { @@ -864,7 +838,7 @@ test "evaluate: multi-base rows that outgrow the buffer are refused, not truncat var small: [8]u8 = undefined; const result = evaluateWith(arena.allocator(), "0xFF + 1", .standard, .{}, &small); try testing.expect(result.is_error); - try testing.expectEqualStrings("error: buffer overflow\n", result.output); + try testing.expectEqualStrings("error: the result is too long to display\n", result.output); } test "the programmer config reaches the result" { @@ -921,7 +895,7 @@ test "conversion: temperature freezing point" { var buf: [256]u8 = undefined; const result = conversion(arena.allocator(), &buf, "0", unit("C"), unit("F")); try testing.expect(!result.is_error); - try testing.expectEqualStrings("0 C = 32 F", result.output); + try testing.expectEqualStrings("0 C = 32 F\n", result.output); } test "conversion: incompatible units is an error" { @@ -940,7 +914,7 @@ test "conversion: alias resolves to canonical name in output" { const result = conversion(arena.allocator(), &buf, "1", unit("kilometer"), unit("meters")); try testing.expect(!result.is_error); // Conversion output now uses the standard comma grouping (NFR-7). - try testing.expectEqualStrings("1 km = 1,000 m", result.output); + try testing.expectEqualStrings("1 km = 1,000 m\n", result.output); } test "conversion: exact conversion prints exactly" { @@ -951,17 +925,17 @@ test "conversion: exact conversion prints exactly" { // The bug this task exists to fix. const feet = conversion(arena.allocator(), &buf, "12", unit("in"), unit("ft")); try testing.expect(!feet.is_error); - try testing.expectEqualStrings("12 in = 1 ft", feet.output); + try testing.expectEqualStrings("12 in = 1 ft\n", feet.output); // Exact affine conversion. const celsius = conversion(arena.allocator(), &buf, "98.6", unit("F"), unit("C")); try testing.expect(!celsius.is_error); - try testing.expectEqualStrings("98.6 F = 37 C", celsius.output); + try testing.expectEqualStrings("98.6 F = 37 C\n", celsius.output); // Exact fractional factor. const mps = conversion(arena.allocator(), &buf, "3.6", unit("km/h"), unit("m/s")); try testing.expect(!mps.is_error); - try testing.expectEqualStrings("3.6 km/h = 1 m/s", mps.output); + try testing.expectEqualStrings("3.6 km/h = 1 m/s\n", mps.output); } test "evaluate: bare conversion expression without subcommand" { @@ -979,7 +953,7 @@ test "evaluate: glued bare conversion" { var buf: [4096]u8 = undefined; const result = evaluateWith(arena.allocator(), "32F to C", .standard, .{}, &buf); try testing.expect(!result.is_error); - try testing.expectEqualStrings("32 F = 0 C", result.output); + try testing.expectEqualStrings("32 F = 0 C\n", result.output); } test "evaluate: conversion value may be an expression" { @@ -988,7 +962,7 @@ test "evaluate: conversion value may be an expression" { var buf: [4096]u8 = undefined; const result = evaluateWith(arena.allocator(), "2*3 kg to g", .standard, .{}, &buf); try testing.expect(!result.is_error); - try testing.expectEqualStrings("6 kg = 6,000 g", result.output); + try testing.expectEqualStrings("6 kg = 6,000 g\n", result.output); } test "evaluate: bare conversion with unknown unit errors" { @@ -1015,7 +989,7 @@ test "evaluate: ordinary expressions are unaffected by conversion detection" { var buf: [4096]u8 = undefined; const result = evaluateWith(arena.allocator(), "2 + 3 * 4", .standard, .{}, &buf); try testing.expect(!result.is_error); - try testing.expectEqualStrings("14", result.output); + try testing.expectEqualStrings("14\n", result.output); } test "evaluate: expression containing a unit-like name still evaluates" { @@ -1066,11 +1040,10 @@ test "amortization: table has a row per period and correct first row" { }, false); try testing.expect(!result.is_error); - // Header, payment, blank, column head, 360 rows, blank, 4 summary lines. - var lines: usize = 0; - var it = std.mem.splitScalar(u8, result.output, '\n'); - while (it.next()) |_| lines += 1; - try testing.expectEqual(@as(usize, 369), lines); + // Header, payment, blank, column head, 360 rows, blank, 4 summary lines, each + // ending in a newline like every other result. + try testing.expectEqual(@as(usize, 369), std.mem.count(u8, result.output, "\n")); + try testing.expect(std.mem.endsWith(u8, result.output, "\n")); try testing.expect(std.mem.indexOf(u8, result.output, "Payment 1,199.10 per period") != null); try testing.expect(std.mem.indexOf(u8, result.output, "1,000.00") != null); @@ -1107,6 +1080,37 @@ test "amortization: bad terms explain themselves" { try testing.expect(std.mem.indexOf(u8, result.output, "loan terms") != null); } +test "every result is complete text, ending in exactly one newline" { + // `main` prints a result as it stands. This held for some outputs and not others: + // single-line answers relied on `main` adding a newline, and programmer mode, + // which carried its own, got a second one and ended in a blank line. + var arena: std.heap.ArenaAllocator = .init(testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + // A buffer each: the results that write into one point into it, so sharing one + // would leave the early results reading the last one's text. + var bufs: [8][4096]u8 = undefined; + for ([_]Result{ + evaluateWith(a, "2 + 2", .standard, .{}, &bufs[0]), + evaluateWith(a, "0xFF + 1", .standard, .{}, &bufs[1]), + evaluateWith(a, "0xFF", .programmer, .{}, &bufs[2]), + evaluateWith(a, "100 km to mi", .standard, .{}, &bufs[3]), + evaluateWith(a, "1 / 0", .standard, .{}, &bufs[4]), + evaluateWith(a, "1 +", .programmer, .{}, &bufs[5]), + conversion(a, &bufs[6], "100", engine.units.findUnit("km").?, engine.units.findUnit("mi").?), + conversion(a, &bufs[7], "abc", engine.units.findUnit("km").?, engine.units.findUnit("mi").?), + amortization(a, .{ .principal = 1000, .rate = 1, .periods = 3 }, false), + amortization(a, .{ .principal = 1000, .rate = 1, .periods = 3 }, true), + amortization(a, .{ .principal = -1, .rate = 1, .periods = 3 }, false), + structLayout(a, "struct { int a; char b; }", struct_layout.default_abi, .plain), + structLayout(a, "struct { int a; char b; }", struct_layout.default_abi, .ansi), + structLayout(a, "struct { bad b; }", struct_layout.default_abi, .plain), + }) |result| { + try testing.expect(std.mem.endsWith(u8, result.output, "\n")); + try testing.expect(!std.mem.endsWith(u8, result.output, "\n\n")); + } +} + // -- Struct layout -- /// What `structLayout` prints for `source` under the profile named `abi`, which must diff --git a/src/main.zig b/src/main.zig index b5b9251..776ceed 100644 --- a/src/main.zig +++ b/src/main.zig @@ -10,9 +10,9 @@ const engine = @import("engine"); const format = @import("cli/format.zig"); const tui = @import("tui.zig"); -/// Parse CLI args and determine the expression and mode. -/// Returns the joined expression and mode, or an error/help output. -pub const ParsedArgs = union(enum) { +/// What the arguments ask for: something to compute, or, when parsing is as far as it +/// goes (help, a usage error), the finished output. +const ParsedArgs = union(enum) { expression: struct { text: []const u8, mode: format.Mode, @@ -39,12 +39,25 @@ pub const ParsedArgs = union(enum) { abi: *const engine.struct_layout.Abi, color: ColorChoice = .auto, }, - output: struct { - text: []const u8, - is_error: bool, - }, + done: format.Result, }; +/// Help that was asked for: stdout, exit status 0. +fn shown(text: []const u8) ParsedArgs { + return .{ .done = .ok(text) }; +} + +/// Arguments that cannot be run, and the usage or message that says so: stderr, +/// exit status 1. +fn refused(text: []const u8) ParsedArgs { + return .{ .done = .{ .output = text, .is_error = true } }; +} + +/// An engine error at the argument layer, worded as everywhere else. +fn failed(err: engine.Error) ParsedArgs { + return .{ .done = .fail(err) }; +} + /// Where `tally struct` reads its definition from. Reading is `main`'s job, since it /// is the part that touches I/O; parsing only decides which. const StructSource = union(enum) { @@ -84,7 +97,7 @@ const ColorChoice = enum { } }; -pub fn parseArgs(allocator: std.mem.Allocator, args: []const []const u8) ParsedArgs { +fn parseArgs(allocator: std.mem.Allocator, args: []const []const u8) ParsedArgs { var mode: format.Mode = .standard; var config: engine.programmer.Config = .{}; var expr_parts = std.ArrayList([]const u8).empty; @@ -112,12 +125,9 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const []const u8) ParsedA if (std.mem.eql(u8, arg, "-p") or std.mem.eql(u8, arg, "--programmer")) { mode = .programmer; } else if (std.mem.eql(u8, arg, "-h") or std.mem.eql(u8, arg, "--help")) { - return .{ .output = .{ - .text = help_text, - .is_error = false, - } }; + return shown(help_text); } else if (std.mem.eql(u8, arg, "--version")) { - return .{ .output = .{ .text = "tally 0.1.0\n", .is_error = false } }; + return shown("tally 0.1.0\n"); } else if (std.mem.eql(u8, arg, "--signed")) { config.signedness = .signed; mode = .programmer; @@ -128,10 +138,10 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const []const u8) ParsedA // Flags that take a value, accepted as either `--bits 8` or `--bits=8`. switch (flagValue(arg, "--bits", args, &i)) { .absent => {}, - .missing => return .{ .output = .{ .text = bits_usage, .is_error = true } }, + .missing => return refused(bits_usage), .value => |text| { config.width = parseBitWidth(text) orelse - return .{ .output = .{ .text = bits_usage, .is_error = true } }; + return refused(bits_usage); // The width only means something in programmer mode: standard // mode is fixed at 64-bit signed. Asking for a width is asking // for programmer mode, so it implies -p rather than erroring. @@ -141,10 +151,10 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const []const u8) ParsedA } switch (flagValue(arg, "--endian", args, &i)) { .absent => {}, - .missing => return .{ .output = .{ .text = endian_usage, .is_error = true } }, + .missing => return refused(endian_usage), .value => |text| { config.display_endian = parseEndian(text) orelse - return .{ .output = .{ .text = endian_usage, .is_error = true } }; + return refused(endian_usage); mode = .programmer; continue; }, @@ -153,20 +163,20 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const []const u8) ParsedA // `--bit 8` used to be evaluated as an expression and reported as an // unexpected token. if (std.mem.startsWith(u8, arg, "--")) { - return .{ .output = .{ .text = unknown_flag_usage, .is_error = true } }; + return refused(unknown_flag_usage); } expr_parts.append(allocator, arg) catch { - return .{ .output = .{ .text = "error: out of memory\n", .is_error = true } }; + return failed(error.OutOfMemory); }; } } if (expr_parts.items.len == 0) { - return .{ .output = .{ .text = "error: no expression provided\n", .is_error = true } }; + return refused("error: no expression provided\n"); } const expression = std.mem.join(allocator, " ", expr_parts.items) catch { - return .{ .output = .{ .text = "error: out of memory\n", .is_error = true } }; + return failed(error.OutOfMemory); }; return .{ .expression = .{ .text = expression, .mode = mode, .config = config } }; @@ -282,13 +292,13 @@ fn parseConvertArgs(allocator: std.mem.Allocator, args: []const []const u8) Pars // a mistake worth showing the usage for rather than parsing. const max_tokens = 4; if (wantsHelp(args)) { - return .{ .output = .{ .text = convert_usage, .is_error = false } }; + return shown(convert_usage); } if (args.len < 2 or args.len > max_tokens) { - return .{ .output = .{ .text = convert_usage, .is_error = true } }; + return refused(convert_usage); } - const joined = std.mem.join(allocator, " ", args) catch return oomArgs(); + const joined = std.mem.join(allocator, " ", args) catch return failed(error.OutOfMemory); defer allocator.free(joined); // The bare form requires a separator word and this one does not, so the tokens are @@ -296,9 +306,9 @@ fn parseConvertArgs(allocator: std.mem.Allocator, args: []const []const u8) Pars // reading is what makes `convert 1 in cm` work, where `in` is the source unit -- // as written, `parseRequest` sees a separator with no unit in front of it. const last = args[args.len - 1]; - const head = std.mem.join(allocator, " ", args[0 .. args.len - 1]) catch return oomArgs(); + const head = std.mem.join(allocator, " ", args[0 .. args.len - 1]) catch return failed(error.OutOfMemory); defer allocator.free(head); - const spelled = std.fmt.allocPrint(allocator, "{s} to {s}", .{ head, last }) catch return oomArgs(); + const spelled = std.fmt.allocPrint(allocator, "{s} to {s}", .{ head, last }) catch return failed(error.OutOfMemory); defer allocator.free(spelled); var failure: ?ParsedArgs = null; @@ -311,35 +321,30 @@ fn parseConvertArgs(allocator: std.mem.Allocator, args: []const []const u8) Pars if (failure == null) failure = reading; } if (failure) |reported| return reported; - return .{ .output = .{ .text = convert_usage, .is_error = true } }; + return refused(convert_usage); } /// One reading of conversion text: the conversion, an error worth reporting, or null /// when the text holds no separator for `parseRequest` to work from. fn readConversion(allocator: std.mem.Allocator, text: []const u8) ?ParsedArgs { const request = engine.units.parseRequest(text) catch |err| { - return .{ .output = .{ .text = format.decoratedError(err), .is_error = true } }; + return failed(err); } orelse return null; // The value has to be a number, and the exact parser is the authority on what one // is: `formatConversion` parses it the same way, so checking here only moves the // report to the argument layer. A shape check would be a second grammar. var probe = engine.Number.parse(allocator, request.value_text) catch { - return .{ .output = .{ .text = "error: invalid number\n", .is_error = true } }; + return failed(error.InvalidNumber); }; probe.deinit(); // `request` borrows from `text`, which the caller frees, so the value text is // copied out. Unit definitions carry static names and need no copy. - const owned = allocator.dupe(u8, request.value_text) catch return oomArgs(); + const owned = allocator.dupe(u8, request.value_text) catch return failed(error.OutOfMemory); return .{ .conversion = .{ .value_text = owned, .from = request.from, .to = request.to } }; } -/// An out-of-memory result at the argument layer. -fn oomArgs() ParsedArgs { - return .{ .output = .{ .text = "error: out of memory\n", .is_error = true } }; -} - /// True when a subcommand's arguments ask for its usage rather than run it. /// /// Every subcommand answers `-h`/`--help` on stdout with exit status 0. The usage @@ -392,7 +397,7 @@ fn parseAmortArgs(args: []const []const u8) ParsedArgs { var round_cents = true; if (wantsHelp(args)) { - return .{ .output = .{ .text = amort_usage, .is_error = false } }; + return shown(amort_usage); } for (args) |arg| { @@ -403,40 +408,37 @@ fn parseAmortArgs(args: []const []const u8) ParsedArgs { } else if (std.mem.eql(u8, arg, "--exact")) { round_cents = false; } else if (std.mem.startsWith(u8, arg, "--")) { - return .{ .output = .{ .text = amort_usage, .is_error = true } }; + return refused(amort_usage); } else { if (count >= positional.len) { - return .{ .output = .{ .text = amort_usage, .is_error = true } }; + return refused(amort_usage); } positional[count] = arg; count += 1; } } - if (count < 3) return .{ .output = .{ .text = amort_usage, .is_error = true } }; + if (count < 3) return refused(amort_usage); const principal = std.fmt.parseFloat(f64, positional[0]) catch { - return .{ .output = .{ .text = "error: invalid number\n", .is_error = true } }; + return failed(error.InvalidNumber); }; const rate_input = std.fmt.parseFloat(f64, positional[1]) catch { - return .{ .output = .{ .text = "error: invalid number\n", .is_error = true } }; + return failed(error.InvalidNumber); }; const periods_input = std.fmt.parseFloat(f64, positional[2]) catch { - return .{ .output = .{ .text = "error: invalid number\n", .is_error = true } }; + return failed(error.InvalidNumber); }; const payment: ?f64 = if (count == 4) std.fmt.parseFloat(f64, positional[3]) catch { - return .{ .output = .{ .text = "error: invalid number\n", .is_error = true } }; + return failed(error.InvalidNumber); } else null; const max_periods: f64 = @floatFromInt(engine.financial.max_schedule_periods); if (!(periods_input >= 1) or periods_input > max_periods or @floor(periods_input) != periods_input) { - return .{ .output = .{ - .text = "error: periods must be a whole number of at least 1\n", - .is_error = true, - } }; + return refused("error: periods must be a whole number of at least 1\n"); } return .{ .amortization = .{ @@ -504,7 +506,7 @@ const color_usage = /// definition; a single argument without one is a path, or `-` for stdin. fn parseStructArgs(allocator: std.mem.Allocator, args: []const []const u8) ParsedArgs { if (wantsHelp(args)) { - return .{ .output = .{ .text = struct_usage, .is_error = false } }; + return shown(struct_usage); } var abi = engine.struct_layout.default_abi; @@ -517,30 +519,30 @@ fn parseStructArgs(allocator: std.mem.Allocator, args: []const []const u8) Parse const arg = args[i]; switch (flagValue(arg, "--abi", args, &i)) { .absent => {}, - .missing => return .{ .output = .{ .text = abi_usage, .is_error = true } }, + .missing => return refused(abi_usage), .value => |id| { abi = engine.struct_layout.findAbi(id) orelse - return .{ .output = .{ .text = abi_usage, .is_error = true } }; + return refused(abi_usage); continue; }, } switch (flagValue(arg, "--color", args, &i)) { .absent => {}, - .missing => return .{ .output = .{ .text = color_usage, .is_error = true } }, + .missing => return refused(color_usage), .value => |when| { color = ColorChoice.parse(when) orelse - return .{ .output = .{ .text = color_usage, .is_error = true } }; + return refused(color_usage); continue; }, } if (std.mem.startsWith(u8, arg, "--")) { - return .{ .output = .{ .text = struct_usage, .is_error = true } }; + return refused(struct_usage); } - parts.append(allocator, arg) catch return oomArgs(); + parts.append(allocator, arg) catch return failed(error.OutOfMemory); } - const found = structSource(allocator, parts.items) catch return oomArgs(); - const source = found orelse return .{ .output = .{ .text = struct_usage, .is_error = true } }; + const found = structSource(allocator, parts.items) catch return failed(error.OutOfMemory); + const source = found orelse return refused(struct_usage); return .{ .layout = .{ .source = source, .abi = abi, .color = color } }; } @@ -628,48 +630,25 @@ pub fn main(init: std.process.Init) u8 { if (args.len <= 1) { tui.run(allocator, io, init.environ_map) catch |err| { - write(io, std.Io.File.stderr(), "error: failed to start TUI: "); - const err_name = @errorName(err); - write(io, std.Io.File.stderr(), err_name); - write(io, std.Io.File.stderr(), "\n"); + const message = std.fmt.allocPrint(allocator, "error: failed to start the TUI: {t}\n", .{err}) catch + "error: failed to start the TUI\n"; + write(io, std.Io.File.stderr(), message); return 1; }; return 0; } - const parsed = parseArgs(allocator, args[1..]); - switch (parsed) { - .output => |out| { - const file = if (out.is_error) std.Io.File.stderr() else std.Io.File.stdout(); - write(io, file, out.text); - return if (out.is_error) @as(u8, 1) else 0; - }, - .expression => |expr| { - var buf: [4096]u8 = undefined; - const result = format.evaluateWith(allocator, expr.text, expr.mode, expr.config, &buf); - const file = if (result.is_error) std.Io.File.stderr() else std.Io.File.stdout(); - write(io, file, result.output); - if (!result.is_error) write(io, std.Io.File.stdout(), "\n"); - return if (result.is_error) @as(u8, 1) else 0; - }, - .conversion => |conv| { - var buf: [4096]u8 = undefined; - const result = format.conversion(allocator, &buf, conv.value_text, conv.from, conv.to); - const file = if (result.is_error) std.Io.File.stderr() else std.Io.File.stdout(); - write(io, file, result.output); - if (!result.is_error) write(io, std.Io.File.stdout(), "\n"); - return if (result.is_error) @as(u8, 1) else 0; - }, - .amortization => |amort| { - const result = format.amortization(allocator, amort.params, amort.summary_only); - const file = if (result.is_error) std.Io.File.stderr() else std.Io.File.stdout(); - write(io, file, result.output); - if (!result.is_error) write(io, std.Io.File.stdout(), "\n"); - return if (result.is_error) @as(u8, 1) else 0; - }, - .layout => |request| { - // The reader has already said what went wrong. - const source = readStructSource(allocator, io, request.source) catch return 1; + var buf: [4096]u8 = undefined; + const result: format.Result = switch (parseArgs(allocator, args[1..])) { + .done => |done| done, + .expression => |expr| format.evaluateWith(allocator, expr.text, expr.mode, expr.config, &buf), + .conversion => |conv| format.conversion(allocator, &buf, conv.value_text, conv.from, conv.to), + .amortization => |amort| format.amortization(allocator, amort.params, amort.summary_only), + .layout => |request| blk: { + const source = switch (readStructSource(allocator, io, request.source)) { + .text => |text| text, + .failed => |failure| break :blk failure, + }; // A terminal check that is cancelled answers no, which errs toward plain. const terminal = std.Io.File.stdout().supportsAnsiEscapeCodes(io) catch false; const paint = request.color.resolve( @@ -677,49 +656,62 @@ pub fn main(init: std.process.Init) u8 { init.environ_map.get("NO_COLOR"), init.environ_map.get("TERM"), ); - const result = format.structLayout(allocator, source, request.abi, paint); - const file = if (result.is_error) std.Io.File.stderr() else std.Io.File.stdout(); - write(io, file, result.output); - return if (result.is_error) @as(u8, 1) else 0; + break :blk format.structLayout(allocator, source, request.abi, paint); + }, + }; + // The one place output is written, and the text is complete: every `Result` + // ends in its own newline. + write(io, if (result.is_error) std.Io.File.stderr() else std.Io.File.stdout(), result.output); + return if (result.is_error) 1 else 0; +} + +/// The text of a struct definition, from wherever the arguments said, or what to +/// print instead. Reading is bounded by the parser's own limit, plus one byte so that +/// a source over it is reported as too long rather than silently cut short. +fn readStructSource(allocator: std.mem.Allocator, io: std.Io, source: StructSource) union(enum) { + text: []const u8, + failed: format.Result, +} { + const limit: std.Io.Limit = .limited(engine.CStructParser.max_source_bytes + 1); + switch (source) { + .text => |text| return .{ .text = text }, + .file => |path| return .{ + .text = std.Io.Dir.cwd().readFileAlloc(io, path, allocator, limit) catch |err| { + return .{ .failed = readFailure(allocator, path, err == error.StreamTooLong, @errorName(err)) }; + }, + }, + .stdin => { + var stdin_buf: [4096]u8 = undefined; + var reader = std.Io.File.stdin().reader(io, &stdin_buf); + return .{ + .text = reader.interface.allocRemaining(allocator, limit) catch |err| { + return .{ .failed = readFailure(allocator, "stdin", err == error.StreamTooLong, @errorName(err)) }; + }, + }; }, } } -/// The text of a struct definition, from wherever the arguments said, or the message -/// to print instead. Reading is bounded by the parser's own limit, plus one byte so -/// that a source over it is reported as too long rather than silently cut short. -fn readStructSource(allocator: std.mem.Allocator, io: std.Io, source: StructSource) error{Failed}![]const u8 { - const limit: std.Io.Limit = .limited(engine.CStructParser.max_source_bytes + 1); - return switch (source) { - .text => |text| text, - .file => |path| std.Io.Dir.cwd().readFileAlloc(io, path, allocator, limit) catch |err| { - return readFailure(allocator, io, path, err == error.StreamTooLong, @errorName(err)); - }, - .stdin => blk: { - var buf: [4096]u8 = undefined; - var reader = std.Io.File.stdin().reader(io, &buf); - break :blk reader.interface.allocRemaining(allocator, limit) catch |err| { - return readFailure(allocator, io, "stdin", err == error.StreamTooLong, @errorName(err)); - }; - }, - }; -} - -/// Report a definition that could not be read, naming the source and the reason. -fn readFailure(allocator: std.mem.Allocator, io: std.Io, what: []const u8, too_long: bool, reason: []const u8) error{Failed} { +/// A definition that could not be read, naming the source and the reason. +fn readFailure(allocator: std.mem.Allocator, what: []const u8, too_long: bool, reason: []const u8) format.Result { const message = if (too_long) std.fmt.allocPrint(allocator, "error: {s}: {s} (the limit is {d} bytes)\n", .{ what, engine.phrase(error.DefinitionTooLong), engine.CStructParser.max_source_bytes, }) else std.fmt.allocPrint(allocator, "error: cannot read {s}: {s}\n", .{ what, reason }); - write(io, std.Io.File.stderr(), message catch "error: out of memory\n"); - return error.Failed; + return .{ .output = message catch return .fail(error.OutOfMemory), .is_error = true }; } +/// Write `msg` at the stream's current position. +/// +/// Streaming rather than `file.writer`'s positional default, which writes from +/// offset 0 of a regular file: with stdout redirected to a file, a second `write` +/// landed on top of the first. `tally convert 100 km mi > out` used to leave +/// "\n00 km = ..." in the file, the result's newline written over its first byte. fn write(io: std.Io, file: std.Io.File, msg: []const u8) void { var buf: [4096]u8 = undefined; - var writer = file.writer(io, &buf); + var writer = file.writerStreaming(io, &buf); writer.interface.writeAll(msg) catch return; writer.interface.flush() catch return; } @@ -859,7 +851,7 @@ test "parseArgs: a bad or incomplete flag value is reported, not evaluated" { // rather than a width of "y". &.{ "--bitsy", "1" }, }) |args| { - try testing.expect(parseArgs(testing.allocator, args).output.is_error); + try testing.expect(parseArgs(testing.allocator, args).done.is_error); } } @@ -874,21 +866,21 @@ test "parseArgs: subcommands answer --help on stdout" { &.{ "struct", "--help" }, &.{ "struct", "-h" }, }) |args| { - const out = parseArgs(testing.allocator, args).output; + const out = parseArgs(testing.allocator, args).done; try testing.expect(!out.is_error); - try testing.expect(std.mem.startsWith(u8, out.text, "usage: tally ")); + try testing.expect(std.mem.startsWith(u8, out.output, "usage: tally ")); } // The same text still reports an error when the arguments are wrong. - try testing.expect(parseArgs(testing.allocator, &.{"convert"}).output.is_error); + try testing.expect(parseArgs(testing.allocator, &.{"convert"}).done.is_error); } test "parseArgs: --help" { const parsed = parseArgs(testing.allocator, &.{"--help"}); switch (parsed) { - .output => |out| { + .done => |out| { try testing.expect(!out.is_error); - try testing.expect(std.mem.startsWith(u8, out.text, "tally")); + try testing.expect(std.mem.startsWith(u8, out.output, "tally")); }, else => unreachable, } @@ -897,9 +889,9 @@ test "parseArgs: --help" { test "parseArgs: --version" { const parsed = parseArgs(testing.allocator, &.{"--version"}); switch (parsed) { - .output => |out| { + .done => |out| { try testing.expect(!out.is_error); - try testing.expectEqualStrings("tally 0.1.0\n", out.text); + try testing.expectEqualStrings("tally 0.1.0\n", out.output); }, else => unreachable, } @@ -908,9 +900,9 @@ test "parseArgs: --version" { test "parseArgs: no expression" { const parsed = parseArgs(testing.allocator, &.{"-p"}); switch (parsed) { - .output => |out| { + .done => |out| { try testing.expect(out.is_error); - try testing.expectEqualStrings("error: no expression provided\n", out.text); + try testing.expectEqualStrings("error: no expression provided\n", out.output); }, else => unreachable, } @@ -971,7 +963,7 @@ test "parseArgs: convert glued negative and decimal value" { test "parseArgs: convert with too few args is usage error" { const parsed = parseArgs(testing.allocator, &.{ "convert", "100" }); switch (parsed) { - .output => |o| try testing.expect(o.is_error), + .done => |o| try testing.expect(o.is_error), else => return error.ExpectedOutput, } } @@ -979,7 +971,7 @@ test "parseArgs: convert with too few args is usage error" { test "parseArgs: convert with too many args is usage error" { const parsed = parseArgs(testing.allocator, &.{ "convert", "1", "m", "ft", "yd" }); switch (parsed) { - .output => |o| try testing.expect(o.is_error), + .done => |o| try testing.expect(o.is_error), else => return error.ExpectedOutput, } } @@ -987,7 +979,7 @@ test "parseArgs: convert with too many args is usage error" { test "parseArgs: convert with non-numeric value errors" { const parsed = parseArgs(testing.allocator, &.{ "convert", "abc", "km", "mi" }); switch (parsed) { - .output => |o| try testing.expect(o.is_error), + .done => |o| try testing.expect(o.is_error), else => return error.ExpectedOutput, } } @@ -1031,7 +1023,7 @@ test "parseArgs: convert handles a glued value followed by a separator" { test "parseArgs: convert rejects unknown units rather than guessing" { const parsed = parseArgs(testing.allocator, &.{ "convert", "1", "smoots", "m" }); switch (parsed) { - .output => |o| try testing.expect(o.is_error), + .done => |o| try testing.expect(o.is_error), else => return error.ExpectedOutput, } } @@ -1042,9 +1034,9 @@ test "parseArgs: convert falls back to the usage text when no reading gets anywh // report and the usage text is all that is left to say. const parsed = parseArgs(testing.allocator, &.{ "convert", "1", "km", "" }); switch (parsed) { - .output => |o| { + .done => |o| { try testing.expect(o.is_error); - try testing.expectEqualStrings(convert_usage, o.text); + try testing.expectEqualStrings(convert_usage, o.output); }, else => return error.ExpectedOutput, } @@ -1098,23 +1090,23 @@ test "parseArgs: amort flags and an explicit payment" { test "parseArgs: amort rejects incomplete or malformed terms" { // Too few terms. const short = parseArgs(testing.allocator, &.{ "amort", "200000", "0.5" }); - try testing.expect(short == .output and short.output.is_error); + try testing.expect(short == .done and short.done.is_error); // Too many. const long = parseArgs(testing.allocator, &.{ "amort", "1", "2", "3", "4", "5" }); - try testing.expect(long == .output and long.output.is_error); + try testing.expect(long == .done and long.done.is_error); // Unknown flag. const bad_flag = parseArgs(testing.allocator, &.{ "amort", "1", "2", "3", "--nope" }); - try testing.expect(bad_flag == .output and bad_flag.output.is_error); + try testing.expect(bad_flag == .done and bad_flag.done.is_error); // Non-numeric term. const bad_number = parseArgs(testing.allocator, &.{ "amort", "abc", "0.5", "360" }); - try testing.expect(bad_number == .output and bad_number.output.is_error); + try testing.expect(bad_number == .done and bad_number.done.is_error); // A fractional period count has no meaning in a schedule. const fractional = parseArgs(testing.allocator, &.{ "amort", "200000", "0.5", "360.5" }); - try testing.expect(fractional == .output and fractional.output.is_error); - try testing.expect(std.mem.indexOf(u8, fractional.output.text, "whole number") != null); + try testing.expect(fractional == .done and fractional.done.is_error); + try testing.expect(std.mem.indexOf(u8, fractional.done.output, "whole number") != null); // Zero periods. const zero = parseArgs(testing.allocator, &.{ "amort", "200000", "0.5", "0" }); - try testing.expect(zero == .output and zero.output.is_error); + try testing.expect(zero == .done and zero.done.is_error); } // -- Struct layout CLI -- @@ -1139,8 +1131,8 @@ test "parseArgs: struct without a brace is a file, and - is stdin" { // Several words and no brace is neither. const neither = parseArgs(testing.allocator, &.{ "struct", "int", "a;" }); - try testing.expect(neither.output.is_error); - try testing.expectEqualStrings(struct_usage, neither.output.text); + try testing.expect(neither.done.is_error); + try testing.expectEqualStrings(struct_usage, neither.done.output); } test "parseArgs: struct --abi picks a profile, in either spelling, anywhere" { @@ -1155,15 +1147,15 @@ test "parseArgs: struct --abi picks a profile, in either spelling, anywhere" { } test "parseArgs: struct reports bad arguments with the usage" { - try testing.expectEqualStrings(abi_usage, parseArgs(testing.allocator, &.{ "struct", "--abi", "vax", "x.h" }).output.text); - try testing.expectEqualStrings(abi_usage, parseArgs(testing.allocator, &.{ "struct", "x.h", "--abi" }).output.text); + try testing.expectEqualStrings(abi_usage, parseArgs(testing.allocator, &.{ "struct", "--abi", "vax", "x.h" }).done.output); + try testing.expectEqualStrings(abi_usage, parseArgs(testing.allocator, &.{ "struct", "x.h", "--abi" }).done.output); for ([_][]const []const u8{ &.{"struct"}, &.{ "struct", "--packed", "x.h" }, }) |args| { - const out = parseArgs(testing.allocator, args).output; + const out = parseArgs(testing.allocator, args).done; try testing.expect(out.is_error); - try testing.expectEqualStrings(struct_usage, out.text); + try testing.expectEqualStrings(struct_usage, out.output); } } @@ -1190,7 +1182,7 @@ test "parseArgs: struct --color takes auto, always or never" { &.{ "struct", "x.h", "--color" }, &.{ "struct", "--color=", "x.h" }, }) |args| { - try testing.expectEqualStrings(color_usage, parseArgs(testing.allocator, args).output.text); + try testing.expectEqualStrings(color_usage, parseArgs(testing.allocator, args).done.output); } }