gronod/iccery-v2-mac
1
0
Fork 0
Code Issues 5 Pull Requests 1 Actions 1 Packages Projects Releases 4 Wiki Activity

Dead UI, duplicated colour types, shared argv / save-panel helpers #86

New Issue
Closed
opened 2026-09-10 18:23:15 +01:00 by gronod · 1 comment
gronod commented 2026-09-10 18:23:15 +01:00
Owner
Copy Link
Copy Source

Summary

Residual duplication after the larger extracts:

  • StagePlaceholderView is M1 leftover (@unknown default only).
  • CIEXYZ / CIELab (ChartreadRow.swift) duplicate XYZColor / LabColor (LabColor.swift).
  • Optional argv flags are copy-pasted trimmingCharacters + empty checks (ColprofArgs, TargenArgs, PrinttargArgs, PrintcalArgs).
  • FileDialogService save panels repeat NSSavePanel setup despite a private open(...) helper.

Spec refs

  • docs/04 / AGENTS.md: do not invent a generic flag table that ignores per-tool -d/-u/-r meanings.
  • #172 profile picker must not accept .ti*.
  • Swatch math: LabColorMath, ColorDifference

Scope

In

  1. Delete Sources/ICCery/StagePlaceholderView.swift. WizardStageContent @unknown default should route to Stage1View (or an assertion-only empty view in DEBUG). Every current WizardStage already has a real screen.

  2. Colour types:

    • Keep one XYZ and one Lab storage type.
    • Preferred: keep CIEXYZ / CIELab as the Codable unkeyed JSON shapes used by ROW_COLORS_JSON, and make XYZColor / LabColor typealiases or wrappers with init(_ cie:) used by LabColorMath.
    • Update ColorDifference / swatch conversion call sites.
    • Do not change JSON keys (XYZ, Lab) or unkeyed encode format.

  3. Tiny ArgsBuilder in Core (internal or public as needed):

enum ArgsBuilder {
    static func option(_ flag: String, _ value: String?) -> [String]
    static func optionIfNonEmpty(_ flag: String, _ value: String?) -> [String]
    static func optionUnlessApprox(_ flag: String, _ value: Double?, skip: Double) -> [String]
    static func flag(_ flag: String, when: Bool) -> [String]
}

Use it inside existing *Args.build methods. Each tool keeps its own enum and validation errors.

  1. FileDialogService: private save(named:extensions:message:startingAt:) used by target save, CSV save, preset export. Dedicated public methods remain (no generic public picker — #103/#210/#211).

  2. Docs touch: add M7 to BUILD-PLAN.md milestone table (CI/mock gate = unit+UI tests listed on these tickets; hardware gate = N/A). Do not rewrite chapters 12–14 (Windows/Linux/TargetPrint spec snapshots stay archival).

Out

  • Merging TargenArgs with CalibrationTargenArgs (different -f / wedge contract). Share -v -d -e -l only via ArgsBuilder, not a shared config.
  • Deleting docs/12–14.
  • ProcessManager / runner extracts (other tickets).

Full solution

  • CalibrationTargenArgs may use ArgsBuilder but keeps invalidSteps 11...51 and -f 0.
  • Typealias approach is acceptable if LabColorMath.xyzToLab can take CIEXYZ without allocation churn in the swatch grid.

Rewrite invariants

  • Per-tool flag semantics unchanged (goldens in TargenTests, PrinttargTests, ColprofArgsTests, PrintcalArgsTests, LpArgsTests).
  • Dedicated file dialogs stay dedicated.
  • No new public “pick any file” API.

Dependencies

Blocks-on: ArgyllRunner extract (avoid a 800-line merge war in ArgyllRunner.swift / argv call sites) and preset mapping (config inits should land before optional-flag cosmetics in the same files).
Unblocks: none.

Test

  • Delete any test that instantiated StagePlaceholderView (none expected).
  • Tests/ICCeryCoreTests/MeasurementTests.swift + color-difference tests: still classify good/warn/fail after the type merge.
  • All *ArgsTests.swift goldens byte-identical.
  • FileHelpersTests / dialog tests if they exist; UI tests that open save panels via UITestHooks should be unchanged (hooks bypass panels).
  • Gamut* tests untouched unless they imported Lab aliases.

Acceptance criteria

  • StagePlaceholderView gone.
  • One XYZ and one Lab type in Core measurement code.
  • Argv goldens unchanged.
  • BUILD-PLAN.md lists M7 + these issue numbers.
  • Core unit tests green.
## Summary Residual duplication after the larger extracts: - `StagePlaceholderView` is M1 leftover (`@unknown default` only). - `CIEXYZ` / `CIELab` (`ChartreadRow.swift`) duplicate `XYZColor` / `LabColor` (`LabColor.swift`). - Optional argv flags are copy-pasted `trimmingCharacters` + empty checks (`ColprofArgs`, `TargenArgs`, `PrinttargArgs`, `PrintcalArgs`). - `FileDialogService` save panels repeat NSSavePanel setup despite a private `open(...)` helper. ## Spec refs - `docs/04` / `AGENTS.md`: **do not** invent a generic flag table that ignores per-tool `-d`/`-u`/`-r` meanings. - `#172` profile picker must not accept `.ti*`. - Swatch math: `LabColorMath`, `ColorDifference` ## Scope **In** 1. Delete `Sources/ICCery/StagePlaceholderView.swift`. `WizardStageContent` `@unknown default` should route to `Stage1View` (or an assertion-only empty view in DEBUG). Every current `WizardStage` already has a real screen. 2. Colour types: - Keep **one** XYZ and one Lab storage type. - Preferred: keep `CIEXYZ` / `CIELab` as the Codable unkeyed JSON shapes used by `ROW_COLORS_JSON`, and make `XYZColor` / `LabColor` typealiases **or** wrappers with `init(_ cie:)` used by `LabColorMath`. - Update `ColorDifference` / swatch conversion call sites. - Do not change JSON keys (`XYZ`, `Lab`) or unkeyed encode format. 3. Tiny `ArgsBuilder` in Core (internal or public as needed): ```swift enum ArgsBuilder { static func option(_ flag: String, _ value: String?) -> [String] static func optionIfNonEmpty(_ flag: String, _ value: String?) -> [String] static func optionUnlessApprox(_ flag: String, _ value: Double?, skip: Double) -> [String] static func flag(_ flag: String, when: Bool) -> [String] } ``` Use it inside existing `*Args.build` methods. **Each tool keeps its own enum and validation errors.** 4. `FileDialogService`: private `save(named:extensions:message:startingAt:)` used by target save, CSV save, preset export. Dedicated public methods remain (no generic public picker — #103/#210/#211). 5. Docs touch: add M7 to `BUILD-PLAN.md` milestone table (CI/mock gate = unit+UI tests listed on these tickets; hardware gate = N/A). Do not rewrite chapters 12–14 (Windows/Linux/TargetPrint spec snapshots stay archival). **Out** - Merging `TargenArgs` with `CalibrationTargenArgs` (different `-f` / wedge contract). Share `-v -d -e -l` only via `ArgsBuilder`, not a shared config. - Deleting `docs/12`–`14`. - ProcessManager / runner extracts (other tickets). ## Full solution - `CalibrationTargenArgs` may use `ArgsBuilder` but keeps `invalidSteps` 11...51 and `-f 0`. - Typealias approach is acceptable if `LabColorMath.xyzToLab` can take `CIEXYZ` without allocation churn in the swatch grid. ## Rewrite invariants - Per-tool flag semantics unchanged (goldens in `TargenTests`, `PrinttargTests`, `ColprofArgsTests`, `PrintcalArgsTests`, `LpArgsTests`). - Dedicated file dialogs stay dedicated. - No new public “pick any file” API. ## Dependencies Blocks-on: ArgyllRunner extract (avoid a 800-line merge war in `ArgyllRunner.swift` / argv call sites) **and** preset mapping (config inits should land before optional-flag cosmetics in the same files). Unblocks: none. ## Test - Delete any test that instantiated `StagePlaceholderView` (none expected). - `Tests/ICCeryCoreTests/MeasurementTests.swift` + color-difference tests: still classify good/warn/fail after the type merge. - All `*ArgsTests.swift` goldens byte-identical. - `FileHelpersTests` / dialog tests if they exist; UI tests that open save panels via `UITestHooks` should be unchanged (hooks bypass panels). - `Gamut*` tests untouched unless they imported Lab aliases. ## Acceptance criteria - [ ] `StagePlaceholderView` gone. - [ ] One XYZ and one Lab type in Core measurement code. - [ ] Argv goldens unchanged. - [ ] `BUILD-PLAN.md` lists M7 + these issue numbers. - [ ] Core unit tests green.
gronod added this to the M7 — Deduplicate & consolidate (develop) milestone 2026-09-10 18:23:15 +01:00
gronod self-assigned this 2026-09-10 18:23:15 +01:00
gronod commented 2026-09-11 15:15:55 +01:00
Author
Owner
Copy Link
Copy Source

Implementation originally landed in stacked commit d4261ba via PR #87.
Completion/verification landed in PR #101 at da602e2.

Acceptance evidence:

[x] Adopted ArgsBuilder helpers across TargenArgs, PrinttargArgs, and PrintcalArgs.

[x] Argv golden outputs asserted byte-for-byte identical with exact flag ordering.

[x] Verified colour types wire-format serialisation as unkeyed arrays with XYZ and Lab keys.

[x] Direct helper coverage added in ArgsBuilderTests.

Verification at milestone/m8-consolidation 891a504ee7:

targeted suites: passed (64 tests)

full universal ICCeryCoreTests: 339 passed, 0 failed

full ICCeryUITests: 29 passed, 0 failed

Closing manually after code and tests are present on the milestone branch.

Implementation originally landed in stacked commit d4261ba via PR #87. Completion/verification landed in PR #101 at da602e2. Acceptance evidence: [x] Adopted ArgsBuilder helpers across TargenArgs, PrinttargArgs, and PrintcalArgs. [x] Argv golden outputs asserted byte-for-byte identical with exact flag ordering. [x] Verified colour types wire-format serialisation as unkeyed arrays with XYZ and Lab keys. [x] Direct helper coverage added in ArgsBuilderTests. Verification at milestone/m8-consolidation 891a504ee722037ab15e01c1ae801a4d4cb9415a: targeted suites: passed (64 tests) full universal ICCeryCoreTests: 339 passed, 0 failed full ICCeryUITests: 29 passed, 0 failed Closing manually after code and tests are present on the milestone branch.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Bug/Architecture
Core / wizard / process infra defect
 Bug/Backend
Argyll orchestration, file/data defect
 Bug/DevOps
Build, packaging, CI defect
 Bug/UI
Views, controls, dialogs defect
 Compat/Breaking
Breaking change that won't be backward compatible
 Feature/Architecture
Core / wizard / process infra
 Feature/Backend
Argyll orchestration, file/data engines
 Feature/DevOps
Build, packaging, CI
 Feature/UI
Views, controls, dialogs
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Project/ICCery-v2
ICCery v2 Mac rewrite
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No labels Feature/Architecture Feature/UI Kind/Enhancement
Priority
Medium
3
 Project/ICCery-v2
Milestone
No items
No Milestone M7 — Deduplicate & consolidate (develop)
Projects
Clear projects
No projects
Assignees
Clear assignees
gronod
No Assignees gronod
1 Participants
Notifications
Due Date
No due date set.
Depends on
#79 Extract ArgyllRunner streaming loop (`runStreamingTool`) and collapse tool-failed errors
gronod/iccery-v2-mac
#82 Map `ProfilingPreset` ↔ targen / printtarg / colprof configs in one place
gronod/iccery-v2-mac
Reference: gronod/iccery-v2-mac#86