From da602e2775613a072548f89b722d6d6659e582d0 Mon Sep 17 00:00:00 2001 From: Gronod Date: Fri, 11 Sep 2026 12:17:52 +0100 Subject: [PATCH] refactor(args): finish shared option helpers (#86) Adopt ArgsBuilder helpers across the remaining argument generators while preserving byte-identical argv and exact flag ordering: - TargenArgs: optionUnlessApprox for -N/-V/-p, optionIfNonEmpty for -c, flag for -G, option for -A. - PrinttargArgs: flag for -r, optionIfNonEmpty for -d and the dynamic -K/-I calibration value (CAL_ protection retained). - PrintcalArgs: flag for -I/-z, optionIfNonEmpty for -a. - ColprofArgs left explicit: FWA and "none" viewing-condition branches cannot be represented by the helpers without changing argv. New ArgsBuilderTests cover nil/present/empty/whitespace/trim, epsilon boundaries, POSIX formatting, and flag handling. Added whitespace-only option cases for targen -c, printtarg -d/-K/-I, and printcal -a, plus chartread row decode and unkeyed XYZ/Lab wire-encoding tests. Refs #86 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../ICCeryCore/Argyll/PrinttargArgs.swift | 14 +-- .../ICCeryCore/Argyll/TargenArgs.swift | 26 ++--- .../ICCeryCore/Profile/PrintcalArgs.swift | 13 +-- Tests/ICCeryCoreTests/ArgsBuilderTests.swift | 94 +++++++++++++++++++ Tests/ICCeryCoreTests/MeasurementTests.swift | 45 +++++++++ Tests/ICCeryCoreTests/PrintcalArgsTests.swift | 23 +++++ Tests/ICCeryCoreTests/PrinttargTests.swift | 17 ++++ Tests/ICCeryCoreTests/TargenTests.swift | 28 ++++++ 8 files changed, 224 insertions(+), 36 deletions(-) create mode 100644 Tests/ICCeryCoreTests/ArgsBuilderTests.swift diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/PrinttargArgs.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/PrinttargArgs.swift index 2d6c08b..e4d966b 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/PrinttargArgs.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/PrinttargArgs.swift @@ -56,23 +56,19 @@ public enum PrinttargArgs { } args.append(contentsOf: ["-R", "\(config.customSeed)"]) case .raster: - args.append("-r") + args.append(contentsOf: ArgsBuilder.flag("-r", when: true)) } - if let label = config.label?.trimmingCharacters(in: .whitespacesAndNewlines), - !label.isEmpty { - args.append(contentsOf: ["-d", label]) - } + args.append(contentsOf: ArgsBuilder.optionIfNonEmpty("-d", config.label)) guard (72...600).contains(config.dpi) else { throw PrinttargArgError.invalidDPI(config.dpi) } args.append(contentsOf: [config.bitDepth.flag, "\(config.dpi)"]) - if !CalibrationIdentity.isCalibration(cleanBasename), - let cal = config.calibrationFile?.trimmingCharacters(in: .whitespacesAndNewlines), - !cal.isEmpty { - args.append(contentsOf: [config.calibrationEmbedOnly ? "-I" : "-K", cal]) + if !CalibrationIdentity.isCalibration(cleanBasename) { + args.append(contentsOf: ArgsBuilder.optionIfNonEmpty( + config.calibrationEmbedOnly ? "-I" : "-K", config.calibrationFile)) } args.append(cleanBasename) diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/TargenArgs.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/TargenArgs.swift index b9c14b8..6ad53ad 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/TargenArgs.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/TargenArgs.swift @@ -70,18 +70,12 @@ public enum TargenArgs { if let n = config.neutralSteps, n > 0 { args.append(contentsOf: ["-n", "\(n)"]) } - if let nConc = config.neutralConcentration, abs(nConc - 0.50) >= 0.001 { - args.append(contentsOf: ["-N", String(format: "%.2f", locale: Locale(identifier: "en_US_POSIX"), nConc)]) - } - if let c = config.preconditioningProfile?.trimmingCharacters(in: .whitespacesAndNewlines), !c.isEmpty { - args.append(contentsOf: ["-c", c]) - } - if config.ofpsHighQuality == true { - args.append("-G") - } - if let a = config.ofpsAdaptation { - args.append(contentsOf: ["-A", String(format: "%.2f", locale: Locale(identifier: "en_US_POSIX"), a)]) - } + args.append(contentsOf: ArgsBuilder.optionUnlessApprox("-N", config.neutralConcentration, skip: 0.50)) + args.append(contentsOf: ArgsBuilder.optionIfNonEmpty("-c", config.preconditioningProfile)) + args.append(contentsOf: ArgsBuilder.flag("-G", when: config.ofpsHighQuality == true)) + args.append(contentsOf: ArgsBuilder.option("-A", config.ofpsAdaptation.map { + String(format: "%.2f", locale: Locale(identifier: "en_US_POSIX"), $0) + })) if let algFlag = config.fullSpreadAlgorithm?.flag { args.append(algFlag) } @@ -91,11 +85,9 @@ public enum TargenArgs { } args.append(contentsOf: ["-l", "\(inkLimit)"]) } - if let v = config.darkEmphasis, abs(v - 1.0) >= 0.001 { - args.append(contentsOf: ["-V", String(format: "%.2f", locale: Locale(identifier: "en_US_POSIX"), v)]) - } - if let p = config.devicePower, p > 0, abs(p - 1.0) >= 0.001 { - args.append(contentsOf: ["-p", String(format: "%.2f", locale: Locale(identifier: "en_US_POSIX"), p)]) + args.append(contentsOf: ArgsBuilder.optionUnlessApprox("-V", config.darkEmphasis, skip: 1.0)) + if let p = config.devicePower, p > 0 { + args.append(contentsOf: ArgsBuilder.optionUnlessApprox("-p", p, skip: 1.0)) } args.append(cleanBasename) diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/PrintcalArgs.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/PrintcalArgs.swift index 6c8c5fa..8ac938b 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/PrintcalArgs.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/PrintcalArgs.swift @@ -79,16 +79,9 @@ public enum PrintcalArgs { var args: [String] = ["-v", "-e"] - if config.noInkLimit { - args.append("-I") - } - if config.verify { - args.append("-z") - } - if let previous = config.previousCalPath?.trimmingCharacters(in: .whitespacesAndNewlines), - !previous.isEmpty { - args.append(contentsOf: ["-a", previous]) - } + args.append(contentsOf: ArgsBuilder.flag("-I", when: config.noInkLimit)) + args.append(contentsOf: ArgsBuilder.flag("-z", when: config.verify)) + args.append(contentsOf: ArgsBuilder.optionIfNonEmpty("-a", config.previousCalPath)) if let tac = config.totalInkLimit, tac > 0 { args.append(contentsOf: ["-m", String(format: "%.1f", tac)]) } else if let tac = config.totalInkLimit { diff --git a/Tests/ICCeryCoreTests/ArgsBuilderTests.swift b/Tests/ICCeryCoreTests/ArgsBuilderTests.swift new file mode 100644 index 0000000..51e9605 --- /dev/null +++ b/Tests/ICCeryCoreTests/ArgsBuilderTests.swift @@ -0,0 +1,94 @@ +import Testing +import Foundation +@testable import ICCeryCore + +@Suite("ArgsBuilder") +struct ArgsBuilderTests { + + // MARK: - option + + @Test("option: nil emits nothing") + func optionNil() { + #expect(ArgsBuilder.option("-f", nil) == []) + } + + @Test("option: present value emits flag and value verbatim") + func optionPresent() { + #expect(ArgsBuilder.option("-f", "abc") == ["-f", "abc"]) + #expect(ArgsBuilder.option("-f", "") == ["-f", ""]) + #expect(ArgsBuilder.option("-f", " padded ") == ["-f", " padded "]) + } + + // MARK: - optionIfNonEmpty + + @Test("optionIfNonEmpty: nil and empty emit nothing") + func optionIfNonEmptyNilEmpty() { + #expect(ArgsBuilder.optionIfNonEmpty("-d", nil) == []) + #expect(ArgsBuilder.optionIfNonEmpty("-d", "") == []) + } + + @Test("optionIfNonEmpty: whitespace-only emits nothing") + func optionIfNonEmptyWhitespace() { + #expect(ArgsBuilder.optionIfNonEmpty("-d", " ") == []) + #expect(ArgsBuilder.optionIfNonEmpty("-d", " \t\n ") == []) + } + + @Test("optionIfNonEmpty: trims surrounding whitespace") + func optionIfNonEmptyTrims() { + #expect(ArgsBuilder.optionIfNonEmpty("-d", " label ") == ["-d", "label"]) + #expect(ArgsBuilder.optionIfNonEmpty("-d", "\tcal.cal\n") == ["-d", "cal.cal"]) + } + + // MARK: - optionUnlessApprox + + @Test("optionUnlessApprox: nil emits nothing") + func optionUnlessApproxNil() { + #expect(ArgsBuilder.optionUnlessApprox("-N", nil, skip: 0.50) == []) + } + + @Test("optionUnlessApprox: exact skip value emits nothing") + func optionUnlessApproxExactSkip() { + #expect(ArgsBuilder.optionUnlessApprox("-N", 0.50, skip: 0.50) == []) + #expect(ArgsBuilder.optionUnlessApprox("-V", 1.0, skip: 1.0) == []) + } + + @Test("optionUnlessApprox: within epsilon emits nothing") + func optionUnlessApproxWithinEpsilon() { + #expect(ArgsBuilder.optionUnlessApprox("-N", 0.5005, skip: 0.50) == []) + #expect(ArgsBuilder.optionUnlessApprox("-V", 0.9995, skip: 1.0) == []) + } + + @Test("optionUnlessApprox: outside epsilon emits flag") + func optionUnlessApproxOutsideEpsilon() { + #expect(ArgsBuilder.optionUnlessApprox("-N", 0.75, skip: 0.50) == ["-N", "0.75"]) + #expect(ArgsBuilder.optionUnlessApprox("-V", 1.50, skip: 1.0) == ["-V", "1.50"]) + #expect(ArgsBuilder.optionUnlessApprox("-N", 0.498, skip: 0.50) == ["-N", "0.50"]) + } + + @Test("optionUnlessApprox: POSIX formatting is locale-stable") + func optionUnlessApproxPOSIX() { + // 1234.5 must never produce a grouping separator or comma decimal. + #expect(ArgsBuilder.optionUnlessApprox("-p", 1234.5, skip: 1.0) == ["-p", "1234.50"]) + #expect(ArgsBuilder.optionUnlessApprox("-p", 2.0, skip: 1.0) == ["-p", "2.00"]) + } + + @Test("optionUnlessApprox: custom epsilon and format honoured") + func optionUnlessApproxCustom() { + #expect(ArgsBuilder.optionUnlessApprox("-x", 1.005, skip: 1.0, epsilon: 0.01) == []) + #expect(ArgsBuilder.optionUnlessApprox("-x", 1.5, skip: 1.0, format: "%.1f") == ["-x", "1.5"]) + } + + // MARK: - flag + + @Test("flag: true emits the bare flag") + func flagTrue() { + #expect(ArgsBuilder.flag("-G", when: true) == ["-G"]) + #expect(ArgsBuilder.flag("-r", when: true) == ["-r"]) + } + + @Test("flag: false emits nothing") + func flagFalse() { + #expect(ArgsBuilder.flag("-G", when: false) == []) + #expect(ArgsBuilder.flag("-r", when: false) == []) + } +} diff --git a/Tests/ICCeryCoreTests/MeasurementTests.swift b/Tests/ICCeryCoreTests/MeasurementTests.swift index ce9f343..3687237 100644 --- a/Tests/ICCeryCoreTests/MeasurementTests.swift +++ b/Tests/ICCeryCoreTests/MeasurementTests.swift @@ -159,6 +159,51 @@ struct ChartreadRowTests { #expect(row.patchCount == 1) #expect(row.patches[0].measured.lab?.l == 51) } + + @Test("Decodes a row carrying both XYZ and Lab arrays") + func decodeXYZAndLab() throws { + let json = """ + {"event": "row_complete", "row_id": "B", "row_index": 1, "total_rows": 2, + "patch_count": 1, "patches": [ + {"id": "7", "loc": "B7", "is_pad": false, "device": [10, 20, 30, 40], + "measured": {"XYZ": [30.5, 32.1, 25.9], "Lab": [63.4, 2.5, -8.2]}} + ]} + """ + let row = try JSONDecoder().decode(ChartreadRow.self, from: Data(json.utf8)) + let measured = row.patches[0].measured + #expect(measured.xyz == CIEXYZ(x: 30.5, y: 32.1, z: 25.9)) + #expect(measured.lab == CIELab(l: 63.4, a: 2.5, b: -8.2)) + } + + @Test("XYZColor/CIEXYZ encode as an unkeyed three-number array") + func xyzWireEncoding() throws { + for color in [XYZColor(x: 1.5, y: 2.5, z: 3.5), CIEXYZ(x: 1.5, y: 2.5, z: 3.5)] { + let value = try JSONSerialization.jsonObject( + with: JSONEncoder().encode(color)) + #expect(value as? [Double] == [1.5, 2.5, 3.5]) + } + } + + @Test("LabColor/CIELab encode as an unkeyed three-number array") + func labWireEncoding() throws { + for color in [LabColor(l: 50, a: -1, b: 2), CIELab(l: 50, a: -1, b: 2)] { + let value = try JSONSerialization.jsonObject( + with: JSONEncoder().encode(color)) + #expect(value as? [Double] == [50, -1, 2]) + } + } + + @Test("PatchColor keeps the XYZ and Lab keys over unkeyed arrays") + func patchColorKeys() throws { + let color = PatchColor( + xyz: CIEXYZ(x: 10, y: 20, z: 30), + lab: CIELab(l: 55, a: 1, b: -2)) + let object = try JSONSerialization.jsonObject( + with: JSONEncoder().encode(color)) as? [String: Any] + #expect(object?["XYZ"] as? [Double] == [10, 20, 30]) + #expect(object?["Lab"] as? [Double] == [55, 1, -2]) + #expect(object?["spectral"] == nil) + } } @Suite("ColourMath") diff --git a/Tests/ICCeryCoreTests/PrintcalArgsTests.swift b/Tests/ICCeryCoreTests/PrintcalArgsTests.swift index e13af6f..57caf9e 100644 --- a/Tests/ICCeryCoreTests/PrintcalArgsTests.swift +++ b/Tests/ICCeryCoreTests/PrintcalArgsTests.swift @@ -44,6 +44,29 @@ struct PrintcalArgsTests { ]) } + @Test("Whitespace-only previous calibration path emits no -a") + func whitespacePreviousCal() throws { + let config = PrintcalConfig( + ti3Basename: "demo", + outputURL: tmp, + previousCalPath: " \n\t " + ) + let args = try PrintcalArgs.build(config: config) + #expect(!args.contains("-a")) + #expect(args == ["-v", "-e", "-o", "/tmp/out.cal", "CAL_demo"]) + } + + @Test("Previous calibration path is trimmed before emission") + func previousCalTrimmed() throws { + let config = PrintcalConfig( + ti3Basename: "demo", + outputURL: tmp, + previousCalPath: " /tmp/old.cal " + ) + let args = try PrintcalArgs.build(config: config) + #expect(args[args.firstIndex(of: "-a")! + 1] == "/tmp/old.cal") + } + @Test("Rejects invalid per-channel limit") func rejectsBadChannelLimit() { let config = PrintcalConfig( diff --git a/Tests/ICCeryCoreTests/PrinttargTests.swift b/Tests/ICCeryCoreTests/PrinttargTests.swift index 440b919..99eec2a 100644 --- a/Tests/ICCeryCoreTests/PrinttargTests.swift +++ b/Tests/ICCeryCoreTests/PrinttargTests.swift @@ -131,6 +131,23 @@ struct PrinttargArgsTests { #expect(!args.contains("-I")) } + @Test("Whitespace-only label emits no -d; whitespace-only calibration emits no -K/-I") + func whitespaceOptions() throws { + let args = try PrinttargArgs.build( + config: config(label: " \n ", calFile: " \t ")) + #expect(!args.contains("-d")) + #expect(!args.contains("-K")) + #expect(!args.contains("-I")) + } + + @Test("Label and calibration values are trimmed before emission") + func trimmedOptions() throws { + let args = try PrinttargArgs.build( + config: config(label: " My Label ", calFile: " /tmp/a.cal ")) + #expect(args[args.firstIndex(of: "-d")! + 1] == "My Label") + #expect(args[args.firstIndex(of: "-K")! + 1] == "/tmp/a.cal") + } + @Test("Unsafe basename throws") func unsafeBasename() { #expect(throws: PathSecurity.Error.self) { diff --git a/Tests/ICCeryCoreTests/TargenTests.swift b/Tests/ICCeryCoreTests/TargenTests.swift index 3329bcc..9929021 100644 --- a/Tests/ICCeryCoreTests/TargenTests.swift +++ b/Tests/ICCeryCoreTests/TargenTests.swift @@ -162,6 +162,34 @@ struct TargenArgsTests { #expect(!args.contains("-p")) } + @Test("Whitespace-only preconditioning profile emits no -c") + func whitespacePreconditioner() throws { + let config = TargenConfig( + colourSpace: .rgb, + patchCount: 800, + whitePatches: 4, + blackPatches: 4, + preconditioningProfile: " \n\t ", + basename: "ws_pre" + ) + let args = try TargenArgs.build(config: config) + #expect(!args.contains("-c")) + } + + @Test("Preconditioning profile is trimmed before emission") + func preconditionerTrimmed() throws { + let config = TargenConfig( + colourSpace: .rgb, + patchCount: 800, + whitePatches: 4, + blackPatches: 4, + preconditioningProfile: " /path/to/profile.icc ", + basename: "trim_pre" + ) + let args = try TargenArgs.build(config: config) + #expect(args[args.firstIndex(of: "-c")! + 1] == "/path/to/profile.icc") + } + @Test("Invalid basename throws") func invalidBasenameThrows() { let config = TargenConfig( -- 2.39.5