Compare commits
5 Commits
cebd41a5ac
...
84e81371c1
| Author | SHA1 | Date | |
|---|---|---|---|
 mixeme
|
84e81371c1 | ||
|
mixeme
|
f11fee10e9 | ||
|
mixeme
|
bde9a2e33e | ||
|
mixeme
|
c5f300b670 | ||
|
mixeme
|
d0dc17a067 |
@@ -25,13 +25,15 @@ creating, grouping, pausing, running, and monitoring scheduled shell commands.
|
||||
- Job definitions stored in a clean, hand-editable `jobs.json`.
|
||||
- `@every` intervals and standard 5-field cron expressions.
|
||||
- Manual and scheduled command runs.
|
||||
- Parallel or sequential execution mode; configurable overlap policy (skip or queue).
|
||||
- Parallel or sequential execution mode; overlap policy (skip or queue) set globally or per job.
|
||||
- Run timeout, off by default, set globally or per job.
|
||||
- Per-run `.log` files with stdout/stderr capture.
|
||||
- Log cleanup by maximum file count and maximum age.
|
||||
- Global pause/resume for scheduled job execution (manual runs remain available).
|
||||
- Desktop notifications on job failure.
|
||||
- Windows tray icon: left-click to show the window, right-click for the menu.
|
||||
- Autostart on login (Windows shortcut; Linux XDG desktop entry).
|
||||
- Detailed or compact job list, and a default or branded theme; both are remembered.
|
||||
|
||||
## Platforms
|
||||
|
||||
@@ -70,10 +72,22 @@ portable application: moving the program folder also moves its configuration.
|
||||
"keep_running_in_tray": true,
|
||||
"notify_on_failure": true,
|
||||
"execution_mode": "parallel",
|
||||
"overlap_policy": "skip"
|
||||
"overlap_policy": "skip",
|
||||
"default_timeout_seconds": 0,
|
||||
"theme": "default",
|
||||
"job_list_view": "detailed"
|
||||
}
|
||||
```
|
||||
|
||||
That is the file GoSentry writes on first run. `default_timeout_seconds` is the
|
||||
run timeout applied to jobs that do not set their own; `0` means no timeout, and
|
||||
it is written out even though it is zero, because a missing value and a
|
||||
deliberate "no timeout" have to stay distinguishable in a hand-edited file.
|
||||
`theme` is `default` or `gosentry` (the branded teal/amber look), and
|
||||
`job_list_view` is `detailed` or `compact` — both are remembered from the
|
||||
choices made in the app. Keys left at their off value (`start_on_login`,
|
||||
`paused`) are omitted until they are turned on.
|
||||
|
||||
`jobs.json` stores job definitions:
|
||||
|
||||
```json
|
||||
@@ -159,8 +173,8 @@ without opening the main window.
|
||||
|
||||
## Queue Settings
|
||||
|
||||
Two settings in the **Queue** group of the Settings tab control how simultaneous
|
||||
and overlapping runs are handled.
|
||||
Three settings in the **Queue** group of the Settings tab control how
|
||||
simultaneous, overlapping, and over-long runs are handled.
|
||||
|
||||
**Execution mode** — applies when multiple jobs become due at the same tick:
|
||||
|
||||
@@ -169,14 +183,25 @@ and overlapping runs are handled.
|
||||
| `parallel` (default) | All due jobs start at the same time. |
|
||||
| `sequential` | Due jobs are started one after another, in the order they appear in the list. |
|
||||
|
||||
**Overlap policy** — applies when a job's next scheduled run fires while its
|
||||
previous run is still active:
|
||||
**Default overlap policy** — applies when a job's next scheduled run fires while
|
||||
its previous run is still active:
|
||||
|
||||
| Value | Behaviour |
|
||||
|-------|-----------|
|
||||
| `skip` (default) | The new run is discarded; the running instance continues. |
|
||||
| `queue` | The new run is held and starts immediately after the current run finishes. |
|
||||
|
||||
**Default timeout (s)** — how long a run may take before it is killed. `0` (the
|
||||
default) means no limit.
|
||||
|
||||
The last two are defaults: a job's own dialog has an **Overlap policy** and a
|
||||
**Timeout (s)** field that override them. A job that overrides nothing follows
|
||||
whatever the Settings tab says, so changing a default moves every such job with
|
||||
it. In `jobs.json` an override is an `overlap_policy` or `timeout_seconds` key
|
||||
on the job; absent means inherit. A `"timeout_seconds": 0` on a job is an
|
||||
override too — it means that job has no timeout even when the global default
|
||||
sets one.
|
||||
|
||||
## Notifications
|
||||
|
||||
When **Notify on failure** is enabled in Settings, GoSentry sends a desktop
|
||||
@@ -197,7 +222,7 @@ Linux:
|
||||
[Desktop Entry]
|
||||
Type=Application
|
||||
Name=GoSentry
|
||||
Exec=/opt/gosentry/gosentry-0.9.0-linux-amd64 --start-in-tray
|
||||
Exec=/opt/gosentry/gosentry-<version>-linux-amd64 --start-in-tray
|
||||
Terminal=false
|
||||
```
|
||||
|
||||
@@ -238,7 +263,7 @@ Known workaround:
|
||||
|
||||
```text
|
||||
dist\windows\
|
||||
gosentry-0.9.0-windows-amd64.exe
|
||||
gosentry-<version>-windows-amd64.exe
|
||||
opengl32.dll
|
||||
...
|
||||
```
|
||||
|
||||
@@ -17,7 +17,7 @@ src/
|
||||
storage/ JSON persistence (gosentry.json, jobs.json)
|
||||
platform/
|
||||
autostart/ Manager interface + Windows (shortcut) and Linux (XDG) impls
|
||||
desktop/ display-scale helper (Linux only)
|
||||
desktop/ desktop entry + icon under XDG data home (Linux only)
|
||||
filemanager/ open a folder in the desktop file manager
|
||||
winproc/ hidden-window startup flags (Windows only)
|
||||
ui/ Fyne windows, tabs, and dialogs; reads service via Events
|
||||
@@ -40,7 +40,7 @@ flowchart LR
|
||||
shell["Platform shell - cmd.exe /C or sh -c"]
|
||||
|
||||
user -->|"edits jobs, settings, runs commands"| ui
|
||||
ui -->|"CreateJob, UpdateJob, DeleteJob, RunNow, UpdateSettings, …"| svc
|
||||
ui -->|"CreateJob, UpdateJob, DeleteJob, RunNow, UpdateSettings, AutostartStatus, …"| svc
|
||||
svc -->|"SaveJobs, SaveConfig, LoadJobs, LoadConfig"| store
|
||||
store -->|"read/write"| config
|
||||
store -->|"read/write"| jobs
|
||||
@@ -54,7 +54,6 @@ flowchart LR
|
||||
svc -->|"emit JobChanged / RunRecorded / JobsLoaded / ErrorOccurred"| ui
|
||||
ui -->|"display jobs, history, status"| user
|
||||
|
||||
ui -->|"SetAutostart, AutostartStatus"| autostart
|
||||
svc -->|"Set / Status via Manager"| autostart
|
||||
```
|
||||
|
||||
@@ -64,7 +63,7 @@ flowchart LR
|
||||
`cmd/gosentry` calls `ui.Run`, which creates an `app.Service`, opens the
|
||||
store, loads `gosentry.json` and `jobs.json`, subscribes the UI to service
|
||||
events, builds the main window, and calls `Service.Start` to begin the
|
||||
scheduler loop. On first launch the service seeds per-job run-time statistics
|
||||
scheduler loop. On every launch the service seeds per-job run-time statistics
|
||||
from existing log files so the details panel reflects accumulated history
|
||||
immediately (see §Statistics below).
|
||||
|
||||
@@ -205,8 +204,9 @@ the moment the window opens.
|
||||
|
||||
### `jobs_view.go` file structure
|
||||
|
||||
`src/ui/jobs_view.go` is split across three files to stay within the ~250-line
|
||||
size guideline:
|
||||
The size guideline for a file in this project is ~250 lines. `src/ui/jobs_view.go`
|
||||
is split across three files along these seams; the view file itself has grown
|
||||
back over the guideline since, and is the next candidate if it grows further:
|
||||
|
||||
| File | Contents |
|
||||
|------|----------|
|
||||
@@ -217,7 +217,8 @@ size guideline:
|
||||
### `settings_view.go` file structure
|
||||
|
||||
`src/ui/settings_view.go` is split across three files the same way, once its
|
||||
own size passed the ~250-line guideline:
|
||||
own size passed the guideline. `src/ui/history_view.go` is over it too and has
|
||||
not been split:
|
||||
|
||||
| File | Contents |
|
||||
|------|----------|
|
||||
|
||||
@@ -2,6 +2,51 @@
|
||||
|
||||
All notable GoSentry changes are recorded in this file.
|
||||
|
||||
## 1.0.0 - 2026-07-27
|
||||
|
||||
**The window opens at the size it asks for, and the Jobs divider can be
|
||||
dragged.**
|
||||
|
||||
**Window:**
|
||||
|
||||
- **The window opens at 1024×660 and can now be dragged narrower than it opens.**
|
||||
Fyne treats the assembled content's minimum size as a hard floor over the
|
||||
requested size, and two widgets in Settings pushed that minimum past 1024 px:
|
||||
a fixed width applied to seven controls that the layout already stretched, and
|
||||
the read-only config path, which grew the whole tab with the length of the
|
||||
path it was showing — a 75-character path alone demanded 1501 px. The path now
|
||||
clips when the window is genuinely narrow instead of widening the window, and
|
||||
the content minimum is 972 px.
|
||||
|
||||
**Jobs:**
|
||||
|
||||
- **The divider between the job list and the details pane is draggable.**
|
||||
Previously the list was pinned at its natural width and the details pane took
|
||||
whatever was left, so a long command or a deep folder path could not be given
|
||||
more room. Either pane can now be widened at the other's expense, and neither
|
||||
can be dragged below its own content, so the details pane condenses rather
|
||||
than clipping. The divider opens at the list's natural width; its position is
|
||||
not saved, so a restart reopens at that default.
|
||||
|
||||
**History:**
|
||||
|
||||
- **Columns measure their own content.** Time, Trigger and State were fixed
|
||||
pixel widths with as little as 1.6 px of headroom and truncated their own
|
||||
values on a scaled UI or at a larger text size; all five now size themselves
|
||||
from the text they have to show, under the current theme. Job and Detail stay
|
||||
bounded so one long row cannot take over the table.
|
||||
|
||||
**Settings:**
|
||||
|
||||
- The **Save / Cancel / Restore defaults** row sits 4 px from the left edge, as
|
||||
its layout always intended, rather than 8.
|
||||
- The caption column is as wide as the widest caption instead of a fixed width,
|
||||
which gives each value column about 22 px more and keeps the captions readable
|
||||
at a larger text size.
|
||||
- The **Application** and **About** blocks are about 2 px tighter: every stacked
|
||||
row group in the app now shares one spacing derived from the theme rather than
|
||||
three separately tuned numbers.
|
||||
|
||||
## 0.15.0 - 2026-07-26
|
||||
|
||||
**Settings points at the jobs file itself, not the folder holding it.**
|
||||
|
||||
+7
-7
@@ -64,7 +64,7 @@ The Windows build is created as a GUI application, so it does not open a termina
|
||||
The binary is written to:
|
||||
|
||||
```text
|
||||
dist\windows\gosentry-0.9.0-windows-amd64.exe
|
||||
dist\windows\gosentry-<version>-windows-amd64.exe
|
||||
```
|
||||
|
||||
### Linux
|
||||
@@ -78,7 +78,7 @@ chmod +x ./scripts/build-linux.sh
|
||||
The binary is written to:
|
||||
|
||||
```text
|
||||
dist/linux/gosentry-0.9.0-linux-amd64
|
||||
dist/linux/gosentry-<version>-linux-amd64
|
||||
```
|
||||
|
||||
### Linux using Docker
|
||||
@@ -94,7 +94,7 @@ chmod +x ./scripts/build-linux-docker.sh
|
||||
The binary is copied to:
|
||||
|
||||
```text
|
||||
dist/linux/gosentry-0.9.0-linux-amd64
|
||||
dist/linux/gosentry-<version>-linux-amd64
|
||||
```
|
||||
|
||||
### Release build from Linux
|
||||
@@ -118,9 +118,9 @@ Non-interactive release builds can pass target names:
|
||||
The binaries are copied to:
|
||||
|
||||
```text
|
||||
dist/linux/gosentry-0.9.0-linux-amd64
|
||||
dist/linux/gosentry-0.9.0-linux-arm64
|
||||
dist/windows/gosentry-0.9.0-windows-amd64.exe
|
||||
dist/linux/gosentry-<version>-linux-amd64
|
||||
dist/linux/gosentry-<version>-linux-arm64
|
||||
dist/windows/gosentry-<version>-windows-amd64.exe
|
||||
```
|
||||
|
||||
### Automated release builds (CI)
|
||||
@@ -195,7 +195,7 @@ CGO_ENABLED=1 go run ./cmd/gosentry
|
||||
- `src/runner` — shell command execution, log file writing, and log cleanup.
|
||||
- `src/storage` — JSON persistence (`gosentry.json`, `jobs.json`).
|
||||
- `src/platform/autostart` — `Manager` interface with Windows (shortcut) and Linux (XDG) implementations.
|
||||
- `src/platform/desktop` — display-scale helper (Linux only).
|
||||
- `src/platform/desktop` — desktop entry and icon under XDG data home (Linux only).
|
||||
- `src/platform/winproc` — hidden-window startup flags (Windows only).
|
||||
- `src/ui` — Fyne windows, tabs, and dialogs; reads service state through events.
|
||||
- `assets` — app icons embedded into the application binary.
|
||||
|
||||
@@ -1,386 +0,0 @@
|
||||
# GUI layout review — custom layouts and composition
|
||||
|
||||
Findings for the *"GUI review — custom layouts and composition"* item in
|
||||
[ROADMAP.md](ROADMAP.md). Scope is composition only: the four custom
|
||||
`fyne.Layout` implementations in [`src/ui/layout.go`](../src/ui/layout.go), the
|
||||
tuned constants the views drive them with, and how the assembled views behave
|
||||
when the window or the theme scale changes. It is not a general code review.
|
||||
|
||||
Reviewed at Fyne v2.7.4. Every number below was measured with a throwaway
|
||||
headless probe (`test.NewApp()` + `theme.DefaultTheme()`), not estimated: at the
|
||||
default text size (14; `theme.Padding()` = 4, `theme.InnerPadding()` = 8) and,
|
||||
where scale matters, at text size 20. The probes were deleted after the
|
||||
measurements were taken; the numbers are reproducible from the recipes quoted in
|
||||
each finding.
|
||||
|
||||
Nothing in the code was changed by this pass. Each finding carries a
|
||||
disposition: **single fix** (goes straight in) or **roadmap** (larger than one
|
||||
fix, comes back to ROADMAP).
|
||||
|
||||
## Summary
|
||||
|
||||
The four custom layouts are mostly justified — but one of them
|
||||
(`compactVBoxLayout`) is a re-implementation of a stock Fyne layout that has
|
||||
existed since 2.5, and another (`minWidthLayout`) is applied at nine sites of
|
||||
which eight enforce a width that turns out to have no visual effect at all.
|
||||
|
||||
The bigger result is about the constants rather than the layouts. Three raw-pixel
|
||||
width constants (`settingsLabelWidth`, `settingsControlWidth`,
|
||||
`minJobsSidebarWidth`) are *floors*, and content already reaches or exceeds all
|
||||
three at the default text size or one step above it. They no longer shape
|
||||
anything on screen. What they still do is set the minimum size of the window —
|
||||
and that minimum (**1165×543** for a typical install) is **wider than the
|
||||
1024×660 window the app asks for at startup**. GoSentry cannot open at its own
|
||||
default size, and the user cannot drag it narrower.
|
||||
|
||||
## Per-layout verdict
|
||||
|
||||
The roadmap asks three questions of each layout: is it still needed, is it the
|
||||
smallest thing that works, does it hold up at other window sizes and theme
|
||||
scales.
|
||||
|
||||
| Layout | Call sites | Still needed | Smallest thing that works | Holds up when scaled |
|
||||
|---|---|---|---|---|
|
||||
| `minWidthLayout` | 9 | Partly — 1 site is load-bearing, 8 are not | No — see F2, F7 | Yes: it takes the max with the child's own minimum, so it can never clip |
|
||||
| `compactVBoxLayout` | 3 | **No** — stock `layout.NewCustomPaddedVBoxLayout` is identical | No — delete the type (F4) | Yes, but the *spacing values* do not (F5) |
|
||||
| `fixedHeightLayout` | 1 | **Yes** — keep | Yes (see below) | Yes: its one caller derives the height from the theme |
|
||||
| `captionValueLayout` | 1 (via `detailRow`, 11 rows) | Yes | Yes | Yes for the caption; the value column has no floor of its own (F9) |
|
||||
|
||||
On `fixedHeightLayout`, which the roadmap singles out as a one-call-site type:
|
||||
there is no stock layout that forces an exact height. The closest stock
|
||||
construction is `container.NewStack(list, rect)` with a transparent
|
||||
`canvas.Rectangle` carrying `SetMinSize(fyne.NewSize(0, activityRowsHeight(3)))`,
|
||||
which is equivalent *here* only because the parent is a `Border` bottom slot and
|
||||
a bottom slot grants exactly `MinSize().Height`. That is a 29-line type traded
|
||||
for a one-line invisible-rectangle trick that depends on the parent staying a
|
||||
`Border`. **Keep the type** — and note the codebase already uses the
|
||||
invisible-rectangle trick twice in `settings_view.go` (F8), so the two idioms
|
||||
currently coexist for no reason. Standardise on the named layout.
|
||||
|
||||
## Per-constant verdict
|
||||
|
||||
| Constant | Value | What content actually needs | Binds? |
|
||||
|---|---|---|---|
|
||||
| `settingsLabelWidth` | 180 | 169.2 — widest caption, *"Default overlap policy"* | Only below text size 15 (180.1 at 15). Visible effect: yes, it sets the caption column |
|
||||
| `settingsControlWidth` | 330 | 306.7 — widest control, the notifications checkbox | Only below text size 16. Visible effect: **none** (F2) |
|
||||
| `minJobsSidebarWidth` | 400 | 448.0 — the 5-button toolbar row | **Never** (F7) |
|
||||
| `detailRowSpacing` | −8 | = −`theme.InnerPadding()` at the default theme | Yes — but the relation to the theme is only implicit (F5) |
|
||||
| `jobRowSpacing` | −8 | as above | as above |
|
||||
| `settingsRowSpacing` | −6 | no principled relation to any theme metric | as above |
|
||||
| `logColumnMinWidth` / `MaxWidth` / `Padding` | 240 / 520 / 24 | a real log name measures 268.9 at text size 14, 373.7 at 20 | Yes; the min/max are sane, the 24 is a guess at `2×InnerPadding` = 16 (F14) |
|
||||
| History column widths | 150/90/170/90/260 | 148.4/75.7/114.9/86.7/144.2 at text size 14 | Yes — Time has 1.6 px of headroom, State 3.3, which a scaled UI eats (F6) |
|
||||
| `activityRowsHeight()` | derived | — | **Correct by construction — this is the model** |
|
||||
| `detailCaptionWidth()` | derived | — | **Correct by construction — this is the model** |
|
||||
|
||||
## Findings
|
||||
|
||||
### F1 — The window cannot open at the size it asks for *(high, roadmap)*
|
||||
|
||||
`run.go:54-56` asks for 1024×660 (or the persisted preference). The assembled
|
||||
content's minimum is **1165.5×542.9** for an install whose config path is 53
|
||||
characters (`C:\Users\alice\AppData\Roaming\GoSentry\gosentry.json`). Fyne
|
||||
enforces that minimum in two places: `window.Resize` takes
|
||||
`size.Max(content.MinSize())`, and `fitContent` calls
|
||||
`view.SetSizeLimits(minWidth, minHeight, …)`. So the window silently opens ~140 px
|
||||
wider than requested and cannot be dragged narrower.
|
||||
|
||||
The height is fine — 543 leaves room on a 720p screen, which is what the
|
||||
condensed details pane was for. The problem is entirely horizontal, and it comes
|
||||
from the Settings tab: `AppTabs` reports the maximum of its children, and the
|
||||
three tabs measure 920.0 (Jobs), 40.0 (History), **1165.5 (Settings)**.
|
||||
|
||||
The Settings minimum is also not a constant — it tracks the length of the config
|
||||
file path: 1040 for a 10-character path, 1165 for 53, **1501 for 75**. A user
|
||||
with a long Windows account name gets a window that will not fit on a 1366×768
|
||||
laptop screen.
|
||||
|
||||
`container.NewVScroll` (`settings_view.go:304`) caps the tab's minimum *height*
|
||||
at 32, and the comment there explains exactly why. The same reasoning was never
|
||||
applied to the width. Two independent causes, F2 and F3, both need fixing to
|
||||
bring the floor under 1024; measured together they take it to **993.3**, at which
|
||||
point the binding row is the *Notifications* checkbox (490.7) and the tab no
|
||||
longer widens with the config path at all.
|
||||
|
||||
This also interacts with the frozen *Window size persistence* roadmap item: a
|
||||
restored width below the content minimum would be silently widened anyway.
|
||||
|
||||
### F2 — `settingsControlWidth` wrappers change nothing but the minimum *(medium, single fix)*
|
||||
|
||||
Seven rows wrap their control in `container.New(minWidthLayout{width: 330}, …)`
|
||||
(`settings_view.go:248, 252, 253, 254, 262, 263, 264`). Measured with a select at
|
||||
three row widths, wrapped versus bare:
|
||||
|
||||
| Row width | Wrapped select size | Bare select size |
|
||||
|---|---|---|
|
||||
| 700 | 516 | 516 |
|
||||
| 514 | 330 | 330 |
|
||||
| 420 | 236 | 236 |
|
||||
|
||||
Identical, at every width, because `settingsRow` puts the control in a `Border`
|
||||
centre slot, which already stretches it to whatever the column gives it — and
|
||||
`minWidthLayout.Layout` in turn resizes its child to the container size. The
|
||||
wrapper's *only* observable effect is on `MinSize`: the row reports 514 instead
|
||||
of 311.9, ×2 columns, which is the 1040 floor in F1.
|
||||
|
||||
**Fix:** delete the constant and the seven wrappers. No visual change at any
|
||||
window size the user can reach; seven containers fewer; the Settings floor drops
|
||||
by 46.7 px (the notifications checkbox then binds at 490.7). `minWidthLayout`
|
||||
itself stays — its remaining caller, the caption box in `settingsRow`, sits in a
|
||||
`Border` *left* slot, which grants exactly `MinSize().Width`, so there the
|
||||
constant is what renders.
|
||||
|
||||
### F3 — One label in Settings is not truncated, and it sets the window width *(medium, single fix)*
|
||||
|
||||
`settingsRow("Config JSON", widget.NewLabel(store.Paths.ConfigPath))`
|
||||
(`settings_view.go:273`) is the only value label in the tab with default
|
||||
wrapping, so its minimum width is the full pixel width of the path: the label
|
||||
measures 392.8 for a 53-character path (12.7 truncated), which makes the row 576.8
|
||||
against 196.7. Fyne's grid gives every column the widest cell's width, so that row
|
||||
alone costs **2×** its width in the tab minimum.
|
||||
|
||||
The rule this row is missing is already established one screen away —
|
||||
`autostartStatus.Wrapping = fyne.TextTruncate` at `settings_view.go:128`, with the
|
||||
comment *"Truncating keeps a long status message from forcing the column wider."*
|
||||
The read-only path row simply never got it.
|
||||
|
||||
**Fix:** truncate it like its neighbour. The full path stays readable whenever the
|
||||
window is wide enough, which it will be by default.
|
||||
|
||||
### F4 — `compactVBoxLayout` re-implements a stock Fyne layout *(medium, single fix)*
|
||||
|
||||
`layout.NewCustomPaddedVBoxLayout(padding float32)` has existed since Fyne 2.5
|
||||
and does exactly what `compactVBoxLayout` does, negative padding included.
|
||||
Compared directly at spacings −8, −6, 0 and 4, the two produce **identical**
|
||||
`MinSize` values and identical position and size for every child; they also agree
|
||||
on a hidden middle child (62.16), an empty object list (0×0) and a single child
|
||||
(43.25×35.08). The stock layout is a strict superset — it additionally
|
||||
distributes `layout.Spacer` objects, which the local copy ignores.
|
||||
|
||||
**Fix:** delete the type (≈40 lines) and replace its three call sites with
|
||||
`container.New(layout.NewCustomPaddedVBoxLayout(spacing), …)`. Keep the *comment*
|
||||
explaining why the spacing is negative — that rationale is not in the stock docs.
|
||||
|
||||
### F5 — The negative spacings are magic numbers with a theme expression available *(medium, single fix)*
|
||||
|
||||
`detailRowSpacing = -8` and `jobRowSpacing = -8` are exactly
|
||||
`-theme.InnerPadding()` at the default theme, and the match is not accidental:
|
||||
two stacked labels contribute 8 px of inner padding each at their shared edge, so
|
||||
−8 removes one label's worth and leaves the other. Written as −8 the reasoning is
|
||||
invisible and the value stops tracking a theme that changes
|
||||
`SizeNameInnerPadding` — the exact class of breakage the roadmap flags.
|
||||
|
||||
Text does not currently overlap at either measured scale (line height 19.1 within
|
||||
a 35.1 label at text size 14; 27.2 within 43.2 at text size 20), so this is a
|
||||
robustness fix, not a rendering bug.
|
||||
|
||||
`settingsRowSpacing = -6` has no such derivation — it is 0.75 of the inner
|
||||
padding, chosen by eye.
|
||||
|
||||
**Fix:** replace the three constants with one function, in the style
|
||||
`activityRowsHeight` and `detailCaptionWidth` already set:
|
||||
|
||||
```go
|
||||
// rowOverlap pulls stacked label rows together by exactly one label's vertical
|
||||
// inner padding, which is the whitespace two adjacent labels double up on.
|
||||
func rowOverlap() float32 { return -theme.InnerPadding() }
|
||||
```
|
||||
|
||||
A function, not a `const`, because `theme.InnerPadding()` must be read after the
|
||||
app exists. Settings then either adopts the same value (−8, a 2 px change) or
|
||||
keeps a documented fraction of it.
|
||||
|
||||
### F6 — History column widths are raw pixels and truncate on a scaled UI *(medium, single fix)*
|
||||
|
||||
`table.SetColumnWidth(0…4, 150/90/170/90/260)` leaves as little as 1.6 px of
|
||||
headroom at the default text size (Time, holding `2026-06-01 10:00:00`) and 3.3
|
||||
(State, holding `Succeeded`). At text size 20 three of the five columns truncate
|
||||
their own content:
|
||||
|
||||
| Column | Width | Needs at text size 20 |
|
||||
|---|---|---|
|
||||
| Time | 150 | 204.9 (`2026-06-01 10:00:00`) |
|
||||
| Trigger | 90 | 101.3 (`Schedule`) |
|
||||
| State | 90 | 117.0 (`Succeeded`) |
|
||||
|
||||
The Log column is already immune — `logColumnWidth` measures its content with
|
||||
`fyne.MeasureText(…, theme.TextSize(), …)`. The remaining five columns should be
|
||||
sized the same way, from a representative sample string per column (a timestamp,
|
||||
the longest trigger and state names) rather than from a pixel count.
|
||||
|
||||
### F7 — `minJobsSidebarWidth` never binds *(low, single fix)*
|
||||
|
||||
The Jobs sidebar's natural minimum is **448.0**, set by the toolbar row of five
|
||||
buttons; the constant is 400. It has never had an effect at the default theme,
|
||||
and scaling only widens the gap. The `Border` left slot gives the sidebar exactly
|
||||
its `MinSize` width, so the wrapper contributes nothing.
|
||||
|
||||
**Fix:** delete the constant and the wrapper. One caveat: `jobsSidebar` in
|
||||
[`jobs_view_test.go`](../src/ui/jobs_view_test.go:141) locates the sidebar by
|
||||
looking for a container whose layout is `minWidthLayout`, so it needs a different
|
||||
anchor in the same change. See also F15 — an `HSplit` would remove the question.
|
||||
|
||||
### F8 — The settings button row is indented twice as far as its comment claims *(low, single fix)*
|
||||
|
||||
`settings_view.go:299-311` builds two transparent `canvas.Rectangle` spacers, the
|
||||
second to *"[indent] the buttons from the left edge the same amount"* as
|
||||
`theme.Padding()`. Measured, the first button lands at **x = 8**, not 4: an
|
||||
`HBox` inserts its own `theme.Padding()` gap *after* the spacer, so the inset is
|
||||
doubled.
|
||||
|
||||
**Fix:** drop both rectangles for the stock
|
||||
`layout.NewCustomPaddedLayout(2*theme.Padding(), 0, theme.Padding(), 0)` wrapped
|
||||
around a plain `HBox`. That reproduces the current 12 px top gap exactly (VBox
|
||||
padding + 2× padding) while giving the intended 4 px left inset — and removes the
|
||||
codebase's second spacing idiom (see the `fixedHeightLayout` note above). If the
|
||||
8 px inset is what was actually wanted, the constant should say so instead.
|
||||
|
||||
### F9 — The details value column has no floor of its own *(low, roadmap)*
|
||||
|
||||
`captionValueLayout` gives the caption a fixed width and hands the remainder to
|
||||
the value, with no lower bound: `valueWidth = size.Width - captionWidth -
|
||||
padding`, clamped at 0. Measured across the pane's reachable widths:
|
||||
|
||||
| Window width | Caption | Value |
|
||||
|---|---|---|
|
||||
| 1583 | 116.2 | 439.3 |
|
||||
| 1024 | 116.2 | 159.8 |
|
||||
| 920 (the panel's own minimum) | 116.2 | 107.8 |
|
||||
|
||||
So the value never actually vanishes — but only because
|
||||
`commandOutputScroll.SetMinSize(fyne.NewSize(460, 70))`
|
||||
([`jobs_view_details.go:62`](../src/ui/jobs_view_details.go:62)) keeps the pane
|
||||
460 px wide, and that constant exists for an unrelated reason (readable command
|
||||
output). Lower it and the value column silently starves; feed the layout 240 px
|
||||
directly and the value renders at width 0 with no warning.
|
||||
|
||||
The coupling is invisible in both files. Either give `captionValueLayout` its own
|
||||
minimum (shrink the caption once the value would drop below some floor, so the
|
||||
caption truncates first), or record the dependency at both ends.
|
||||
|
||||
Related, same type: `MinSize` and `Layout` both `return` silently when
|
||||
`len(objects) != 2`. A miswired caller renders an empty row rather than failing.
|
||||
The type is package-private with one constructor (`detailRow`), so this is a
|
||||
documentation-grade nit, not a defect.
|
||||
|
||||
### F10 — The detail caption list is written twice *(low, single fix)*
|
||||
|
||||
`detailCaptionWidth()` ([`jobs_view_details.go:165`](../src/ui/jobs_view_details.go:165))
|
||||
hard-codes the eleven caption strings to measure the widest; `container()` writes
|
||||
the same eleven strings again, thirty lines above, to build the rows. They match
|
||||
today. Add a twelfth row and forget the list, and the new caption silently
|
||||
truncates — with no test to catch it, because the width is correct for the
|
||||
captions the function knows about.
|
||||
|
||||
**Fix:** build the rows from one `[]struct{caption string; value fyne.CanvasObject}`
|
||||
and derive the width from that same slice.
|
||||
|
||||
### F11 — The History table re-sorts its whole backing slice once per cell *(medium, single fix)*
|
||||
|
||||
```go
|
||||
func(id widget.TableCellID, item fyne.CanvasObject) {
|
||||
label.SetText(historyCellText(id, sortedEvents()))
|
||||
```
|
||||
|
||||
`sortedEvents()` copies the event slice and `sort.SliceStable`s it — and it is
|
||||
called from the per-cell update callback. Measured with 300 events in a 1200×800
|
||||
window: **126 `UpdateCell` calls per `Refresh`, so 126 copies and 126 sorts of a
|
||||
300-element slice** for one redraw. A redraw runs on every recorded run, every
|
||||
service error, and every UI action, since `mainwindow.go`'s observer calls
|
||||
`refresh()` unconditionally.
|
||||
|
||||
**Fix:** sort once per refresh into a slice the callback reads. The same callback
|
||||
also assigns a constant `fyne.TextStyle{}` that the row template already carries,
|
||||
then calls `label.Refresh()` for that assignment; with the assignment gone the
|
||||
`Refresh` goes too, because `SetText` already refreshes.
|
||||
|
||||
### F12 — Jobs handlers repeat the work `refreshView` is about to do *(low, single fix)*
|
||||
|
||||
`refreshView` already calls `syncFromService()`, recomputes `filteredJobs`,
|
||||
updates the details panel and calls `list.Refresh()`. Six handlers call
|
||||
`list.Refresh()` immediately before calling `refreshView()`
|
||||
(`jobs_view.go:238, 256, 270, 302, 315, 347`, plus `181`/`191` in the folder
|
||||
filter), and the pause handler additionally repeats `syncFromService()`.
|
||||
`widget.List.Refresh()` re-creates the row template and re-measures the row
|
||||
height, so this is not free. Where the earlier `syncFromService()` is genuinely
|
||||
needed — `folderOptions(jobs)` reads the refreshed slice — it should stay, with
|
||||
only the redundant `list.Refresh()` removed.
|
||||
|
||||
### F13 — `settings_view.go` is 445 lines and speaks two dialects *(low, roadmap)*
|
||||
|
||||
Past the ~250-line guideline in [ARCHITECTURE.md](ARCHITECTURE.md), and the split
|
||||
`jobs_view.go` already demonstrates the shape. Three seams are visible in the
|
||||
file as it stands:
|
||||
|
||||
- **`settings_view.go`** — `settingsView`: field construction, save/load/validate.
|
||||
- **`settings_view_layout.go`** — `settingsSection`, `settingsRow`, the two
|
||||
columns, the button row.
|
||||
- **`settings_view_helpers.go`** — `fyneVersion`, `mustParseURL`,
|
||||
`settingsFolderPath`, `openFolder`, the file/folder pickers.
|
||||
|
||||
Two composition inconsistencies to settle in the same pass rather than carry
|
||||
across the split:
|
||||
|
||||
- The *Application* and *About* blocks use `settingsSection` (condensed spacing);
|
||||
*Queue* and *Storage* inline `container.NewVBox(header, rows…)` (theme
|
||||
spacing). Two spellings of "a titled block of rows" in one function. Both
|
||||
comments justify the difference, but a `settingsSection(title, spacing, rows…)`
|
||||
— or two named constructors — would say it once.
|
||||
- `chooseFile` and `chooseJSONFile` are the same eight lines apart from
|
||||
`SetFilter`. One function taking a filter (`nil` for none) removes the copy.
|
||||
Note `chooseFile` is also used by `job_dialog.go`, so it belongs with the
|
||||
helpers, not with Settings.
|
||||
|
||||
### F14 — `logColumnPadding = 24` *(low, single fix)*
|
||||
|
||||
The cell is a `widget.Label`, whose text is inset by `theme.InnerPadding()` on
|
||||
each side: 16 px, plus table padding. 24 is a hand-tuned guess at that. Express
|
||||
it as `2*theme.InnerPadding()` (plus whatever margin is wanted, named) so it
|
||||
tracks the theme like the width it is added to already does. The three constants
|
||||
are also untyped `int` while every other width constant in the package is a typed
|
||||
`float32`.
|
||||
|
||||
### F15 — Master/detail is a fixed `Border`, not a split *(low, roadmap)*
|
||||
|
||||
`jobs_view.go:365-366` pins the sidebar at its `MinSize` in a `Border` left slot,
|
||||
so the user can never give the details pane more room or the job list less. This
|
||||
is what `container.NewHSplit` is for: a draggable divider, `SetOffset` for the
|
||||
initial ratio, and no `minWidthLayout` wrapper or `minJobsSidebarWidth` constant
|
||||
needed. It changes behaviour rather than just structure, so it is a roadmap item,
|
||||
not a cleanup — but it subsumes F7 outright, gives F9 a user-controlled escape
|
||||
(drag the divider left to widen the value column), and is the idiomatic Fyne
|
||||
composition for this screen.
|
||||
|
||||
## What holds up — do not "fix" these
|
||||
|
||||
- **`activityRowsHeight()` and `detailCaptionWidth()`.** Both derive their result
|
||||
by measuring a real widget under the current theme. They are the pattern
|
||||
everything else in the package should converge on, and they already behave
|
||||
correctly at text size 20 (114.2→138.7 and 116.2→159.1).
|
||||
- **`minWidthLayout` degrades safely.** Because `MinSize` takes the *max* of the
|
||||
configured width and the child's own minimum, a width routed through this layout
|
||||
can never clip its content — it can only inflate a minimum. That is why F1 is a
|
||||
sizing problem and not a rendering one, and it is the property the History
|
||||
column widths in F6 lack.
|
||||
- **The compact/detailed row mechanism.** `applyRowMode` expressing the mode as
|
||||
visibility, with the comment about `widget.List` caching the template's
|
||||
`MinSize`, is correct and non-obvious; both call sites are needed.
|
||||
- **The negative spacing itself.** It does not overlap text at either measured
|
||||
scale. F5 is about how the number is written, not about abandoning the
|
||||
technique.
|
||||
- **`container.NewVScroll` around Settings.** It correctly keeps the tab from
|
||||
dictating the window's minimum height. F1 is the same idea left half-applied.
|
||||
|
||||
## Suggested order
|
||||
|
||||
1. F2 + F3 together — they are the two causes of F1, and neither changes anything
|
||||
visible above the minimum window width. Measured result: 1165.5 → **993.3**.
|
||||
Verify by asserting the assembled content's `MinSize().Width` stays under the
|
||||
1024 default in a test; that is the regression guard F1 has been missing. If
|
||||
more headroom is wanted afterwards, deriving `settingsLabelWidth` from the
|
||||
widest caption the way `detailCaptionWidth` does buys another 21.7 (→ 971.7).
|
||||
2. F4, F7, F14 — deletions, no behaviour change. F7 needs the test helper
|
||||
re-anchored in the same commit.
|
||||
3. F11, F12 — redraw cost, self-contained.
|
||||
4. F5, F8, F10 — the workaround-shaped constants, once the deletions have settled.
|
||||
5. F6 — needs a per-column sample string decided first.
|
||||
6. F9, F13, F15 — back to [ROADMAP.md](ROADMAP.md).
|
||||
@@ -1,477 +0,0 @@
|
||||
# Implementation plan — GUI layout cleanup
|
||||
|
||||
## Context
|
||||
|
||||
[GUI-LAYOUT-REVIEW.md](GUI-LAYOUT-REVIEW.md) recorded fifteen findings against
|
||||
the `ui` package's custom layouts, tuned constants, and view composition. This
|
||||
plan turns all fifteen into landable work.
|
||||
|
||||
**Part A** (stages 1–6) is the set the review classified as single fixes: each
|
||||
stage is one commit, none of them changes what the user sees except where the
|
||||
stage says so. **Part B** (stages 7–8) is the roadmap-sized work — a file split
|
||||
and a behaviour change. **Stage 9** closes the roadmap item and ships the batch.
|
||||
|
||||
Three decisions were open when this plan was written; all three are settled and
|
||||
folded in below:
|
||||
|
||||
- **Scope: everything.** Part A, the `settings_view.go` split (stage 7) and the
|
||||
draggable split pane (stage 8) all land, so the roadmap item closes completely
|
||||
rather than carrying F9, F13 and F15 forward.
|
||||
- **The divider position is not persisted** (stage 8). It stays a `ui`-only
|
||||
change with the initial ratio computed at build time; no new `Config` field.
|
||||
- **Row spacing unifies on −8** (stage 2), so `rowOverlap()` is the single
|
||||
expression for the whole package.
|
||||
|
||||
Finding IDs (F1…F15) refer to the review. One item found while writing this plan
|
||||
and not in the review is labelled N1.
|
||||
|
||||
## Design decisions
|
||||
|
||||
These are cross-cutting; settling them once keeps the stages from contradicting
|
||||
each other.
|
||||
|
||||
- **A size that must track the theme is a function, not a `const`.**
|
||||
`theme.Padding()` and friends read `fyne.CurrentApp()`, so they cannot be
|
||||
evaluated at package init. `activityRowsHeight()` and `detailCaptionWidth()`
|
||||
already establish the shape — measure a real widget under the current theme,
|
||||
at build time. Every constant this plan replaces converges on that form, and
|
||||
Stage 9 writes the rule into [STANDARDS.md](STANDARDS.md).
|
||||
- **One spacing idiom: named layouts.** The transparent-`canvas.Rectangle`
|
||||
spacer disappears (F8). Where an exact size is needed and no stock layout
|
||||
expresses it, a named `fyne.Layout` in `layout.go` does — which is why
|
||||
`fixedHeightLayout` stays.
|
||||
- **`minWidthLayout` stays, its callers mostly do not.** The type is sound (it
|
||||
takes the max, so it can never clip). Seven of its nine call sites are inert
|
||||
(F2) and one never binds (F7); the survivor is the settings caption box, and
|
||||
even that stops being a raw pixel count in Stage 6.
|
||||
- **The assembled window must fit the size the app asks for.** `run.go` opens at
|
||||
1024×660; Fyne treats the content minimum as a hard floor. That becomes an
|
||||
invariant with a test behind it (Stage 1), not a thing to re-measure by hand.
|
||||
- **"No visual change" is asserted, not claimed.** Stages 1 and 3 delete widths
|
||||
on the grounds that nothing renders differently. Each carries a test that
|
||||
measures geometry rather than trusting the argument in this document.
|
||||
- **No new dependencies.** Everything here is stock Fyne 2.7.4 plus the standard
|
||||
library.
|
||||
|
||||
## Part A — single fixes
|
||||
|
||||
### Stage 1 — bring the window minimum under the default window size (F1, F2, F3)
|
||||
|
||||
The headline finding: assembled content measures 1165.5×542.9 against a
|
||||
requested 1024×660, so Fyne silently widens the window and forbids dragging it
|
||||
back. Two causes, both in `settings_view.go`.
|
||||
|
||||
**Changes**
|
||||
|
||||
1. `src/ui/settings_view.go` — **F2**: delete `settingsControlWidth`
|
||||
(line 30) and unwrap the seven controls that use it (lines 248, 252, 253,
|
||||
254, 262, 263, 264), e.g. `settingsRow("Theme", themeSelect)`. `settingsRow`
|
||||
puts the control in a `Border` centre slot, which already stretches it to the
|
||||
column width, so the wrapper only ever inflated `MinSize`.
|
||||
2. `src/ui/settings_view.go:273` — **F3**: give the read-only config-path label
|
||||
`Truncation = fyne.TextTruncateClip`, the non-deprecated form established by
|
||||
commit 706aa8e. Extract it to a local so the assignment has somewhere to
|
||||
live. This is what stops the tab's minimum from tracking the length of the
|
||||
user's config path (1040 → 1165 → 1501 for 10-, 53- and 75-character paths).
|
||||
3. `src/ui/run.go:54-55` — replace the `1024` / `660` literals with
|
||||
`defaultWindowWidth` / `defaultWindowHeight` consts so the test in this stage
|
||||
asserts against the same numbers the app uses, with a comment recording that
|
||||
Fyne enforces the content minimum over them.
|
||||
|
||||
**Tests**
|
||||
|
||||
- `src/ui/mainwindow_test.go` — `TestMainViewFitsTheDefaultWindowSize`: build
|
||||
`newMainView` against a store whose `Paths.ConfigPath` is a deliberately long
|
||||
path, then assert `content.MinSize()` is within
|
||||
`defaultWindowWidth`×`defaultWindowHeight`. This is the regression guard F1
|
||||
never had; the long path is what keeps F3 from silently regressing.
|
||||
- `src/ui/settings_view_test.go` — `TestSettingsRowStretchesItsControl`: resize a
|
||||
`settingsRow` to a width above its minimum and assert the control fills the
|
||||
remaining width. That is the property that makes F2's deletion invisible.
|
||||
|
||||
**Expected result:** Settings minimum width 1165.5 → 993.3; the binding row
|
||||
becomes the *Notifications* checkbox, and the tab no longer widens with the
|
||||
config path.
|
||||
|
||||
### Stage 2 — stock layout, theme-derived spacing (F4, F5)
|
||||
|
||||
**Changes**
|
||||
|
||||
1. `src/ui/layout.go` — delete `compactVBoxLayout` (lines 40–80). Verified
|
||||
identical to `layout.NewCustomPaddedVBoxLayout` at spacings −8, −6, 0 and 4,
|
||||
for the three-child, hidden-middle-child, single-child and empty cases; the
|
||||
stock layout additionally handles `layout.Spacer`, which the copy ignored.
|
||||
2. `src/ui/jobs_view.go:139`, `src/ui/jobs_view_details.go:124`,
|
||||
`src/ui/settings_view.go:323` — switch to
|
||||
`container.New(layout.NewCustomPaddedVBoxLayout(rowOverlap()), …)`. Add the
|
||||
`fyne.io/fyne/v2/layout` import to the latter two.
|
||||
3. `src/ui/layout.go` — **F5**: add `rowOverlap()` and delete
|
||||
`detailRowSpacing`, `jobRowSpacing` (`jobs_view.go:29,35`) and
|
||||
`settingsRowSpacing` (`settings_view.go:36`):
|
||||
|
||||
```go
|
||||
// rowOverlap is the (negative) gap that pulls stacked label rows together by
|
||||
// exactly one label's vertical inner padding. Two adjacent labels each inset
|
||||
// their text by theme.InnerPadding(), so the whitespace between two lines of
|
||||
// text is double what a single row needs; removing one label's worth
|
||||
// condenses the block without letting the text lines touch. Derived rather
|
||||
// than hard-coded so it follows a theme that changes SizeNameInnerPadding.
|
||||
func rowOverlap() float32 { return -theme.InnerPadding() }
|
||||
```
|
||||
|
||||
This carries over the explanation the deleted `compactVBoxLayout` comment
|
||||
held, which is the part not documented upstream.
|
||||
|
||||
**Decided:** Settings currently uses −6 where the other two use −8; adopting
|
||||
`rowOverlap()` moves it to −8, a 2 px tightening of the Application and About
|
||||
blocks, and that is the one deliberate visual change in Part A. Check it in the
|
||||
running app at step 4 of *Verification*; if it reads too tight, the fallback is a
|
||||
documented fraction (`rowOverlap() * 0.75`), never a reinstated literal.
|
||||
|
||||
**Tests**
|
||||
|
||||
- `src/ui/layout_test.go` (new) — `TestRowOverlapMatchesInnerPadding`: assert
|
||||
`rowOverlap() == -theme.InnerPadding()` and that it is negative, under the
|
||||
default theme and under a theme with a different inner padding.
|
||||
- Existing `TestJobListViewToggleShrinksRowsAndPersists` already covers that the
|
||||
job rows still shrink in compact mode through the replacement layout; no new
|
||||
test needed for F4 beyond a green run.
|
||||
|
||||
### Stage 3 — delete the sidebar width floor (F7)
|
||||
|
||||
**Changes**
|
||||
|
||||
1. `src/ui/jobs_view.go:19` — delete `minJobsSidebarWidth`; at line 365 pass
|
||||
`sidebar` straight to the `Border` left slot. The toolbar row already needs
|
||||
448 against the constant's 400, so it has never bound, and a `Border` left
|
||||
slot renders the child at exactly its `MinSize` width either way.
|
||||
2. `src/ui/jobs_view_test.go:141` — re-anchor `jobsSidebar`, which currently
|
||||
finds the sidebar by looking for a `minWidthLayout` container. Anchor it on
|
||||
the pane structure instead: the `Border`'s left object is the sidebar
|
||||
(`panel.Objects[1]` — `NewBorder` appends slots after the centre object).
|
||||
Add the same one-line comment the row-template code uses about `NewBorder`
|
||||
ordering, since this is the second place that ordering is relied on. Stage 8
|
||||
moves this anchor once more, to the split's `Leading`.
|
||||
|
||||
**Tests**
|
||||
|
||||
- `src/ui/jobs_view_test.go` — `TestJobsSidebarWidthIsItsContent`: assert the
|
||||
sidebar's `MinSize().Width` equals the toolbar row's, i.e. that nothing else
|
||||
imposes a floor. The existing `jobsList`/`jobsViewToggle` helpers exercise the
|
||||
re-anchored lookup.
|
||||
|
||||
### Stage 4 — redraw cost (F11, F12)
|
||||
|
||||
**Changes**
|
||||
|
||||
1. `src/ui/history_view.go:133-148` — **F11**: hoist the sort out of the per-cell
|
||||
callback. Keep a `rows []event` snapshot beside `descending`; a `resort()`
|
||||
closure refills it. Call `resort()` when the view is built, from `refresh()`
|
||||
(line 179) before `table.Refresh()`, and from the header tap handler
|
||||
(line 157) after flipping `descending`. **The length callback must switch
|
||||
from `len(*events)` to `len(rows)`** — today it is safe only because each
|
||||
cell re-derives the sorted slice from the same source, and a cache breaks
|
||||
that agreement. Cells then read `rows` directly.
|
||||
Drop the `label.TextStyle = fyne.TextStyle{}` assignment (the template
|
||||
already carries the zero value) and the `label.Refresh()` that exists for it,
|
||||
since `SetText` refreshes.
|
||||
2. `src/ui/history_view.go:109-121` — hoist the `headers` slice out of
|
||||
`headerText` into a package-level `var historyHeaders = [...]string{…}`; it
|
||||
is currently reallocated on every header-cell update.
|
||||
3. `src/ui/jobs_view.go` — **F12**: delete the `list.Refresh()` calls that
|
||||
immediately precede `refreshView()` (lines 238, 256, 270, 302, 315, 347) and
|
||||
the duplicate `syncFromService()` at line 314. `refreshView` already does
|
||||
both. Keep the `syncFromService()` calls at 228, 253 and 333 — `folderOptions(jobs)`
|
||||
reads the refreshed slice on the next line.
|
||||
**Careful with line 181:** the folder-filter handler returns early when the
|
||||
filter matches nothing, and that path never reaches `refreshView()`, so its
|
||||
`list.Refresh()` is load-bearing. Move it into the early-return branch rather
|
||||
than deleting it.
|
||||
|
||||
**Tests**
|
||||
|
||||
- `src/ui/history_view_test.go` — `TestHistorySortToggleKeepsRowsInSync`: build
|
||||
the view over N events, flip the sort through the header tap, assert the first
|
||||
and last cell text; append an event, call `refresh`, assert the row count and
|
||||
the new event's placement in both orders. This is the regression test for the
|
||||
cache-versus-length hazard the change introduces (severity medium →
|
||||
regression test required by [STANDARDS.md](STANDARDS.md)).
|
||||
- `src/ui/jobs_view_test.go` — extend an existing button test to assert the
|
||||
selection and details survive a handler that lost its `list.Refresh()`.
|
||||
|
||||
**Measured motivation:** 300 events in a 1200×800 window produced 126
|
||||
`UpdateCell` calls per `Refresh`, each copying and sorting the whole 300-element
|
||||
slice, on every recorded run.
|
||||
|
||||
### Stage 5 — content-measured History columns (F6, F14)
|
||||
|
||||
The Log column already sizes itself from its content; the other five are pixel
|
||||
counts with as little as 1.6 px of headroom, and three of them truncate their own
|
||||
values at text size 20.
|
||||
|
||||
**Changes**
|
||||
|
||||
1. `src/ui/history_view.go:45-74` — generalise `logColumnWidth` into one helper:
|
||||
|
||||
```go
|
||||
// textColumnWidth returns the width a table column needs to show the widest
|
||||
// of the given samples in full, measured under the current theme so it
|
||||
// follows text size and DPI, and clamped to [min, max].
|
||||
func textColumnWidth(samples []string, min, max float32) float32
|
||||
```
|
||||
|
||||
with `cellPadding()` = `2 * theme.InnerPadding()` replacing the hand-tuned
|
||||
`logColumnPadding = 24` (**F14**), and the min/max bounds themselves expressed
|
||||
as measured text (`textWidth(strings.Repeat("0", 30))`) rather than raw
|
||||
pixels. The three surviving bounds become typed `float32`, matching every
|
||||
other width in the package.
|
||||
2. `src/ui/history_view.go:169-174` — feed each column its samples:
|
||||
- **Time** — the timestamp format itself (`2006-01-02 15:04:05` rendered), a
|
||||
fixed width; no content scan needed.
|
||||
- **Trigger** — the known trigger strings (`Schedule`, `Manual`, `UI`,
|
||||
`Unknown`), a closed set.
|
||||
- **State** — the known state strings (`Succeeded`, `Failed`, `Started`,
|
||||
`Error`, `Jobs loaded`), likewise closed.
|
||||
- **Job** and **Detail** — free text: measure the values actually present,
|
||||
bounded like the Log column so one long row cannot dominate the table.
|
||||
3. `refresh` (line 179) recomputes all content-derived columns, not just Log.
|
||||
|
||||
**Tests**
|
||||
|
||||
- `src/ui/history_view_test.go` — `TestHistoryColumnsFitTheirContent`: for each
|
||||
column, at the default text size and at a scaled theme, assert the configured
|
||||
width is at least what its widest sample measures. Table-driven, and it fails
|
||||
today for Time, Trigger and State at text size 20.
|
||||
- `TestTextColumnWidthClamps`: below-min, in-range and above-max samples.
|
||||
|
||||
**Note:** the trigger and state strings are produced in `app`/`runner`
|
||||
(`app.StatusText`, the run recorder). The samples live in `ui` next to the
|
||||
column they size; a comment should point at where the real strings come from so
|
||||
a new state gets added in both places.
|
||||
|
||||
### Stage 6 — one caption-width helper, button row, truncation idiom (F8, F10, N1, F9 nit)
|
||||
|
||||
**Changes**
|
||||
|
||||
1. `src/ui/layout.go` — **F10**: one helper for both views:
|
||||
|
||||
```go
|
||||
// captionColumnWidth returns the width to reserve for a column of bold
|
||||
// captions: the widest of them, measured under the current theme so it tracks
|
||||
// text size and DPI instead of a hand-tuned constant.
|
||||
func captionColumnWidth(captions ...string) float32
|
||||
```
|
||||
|
||||
2. `src/ui/jobs_view_details.go:119-191` — build the metadata rows and their
|
||||
width from **one** list instead of two. Today `detailCaptionWidth` hard-codes
|
||||
the eleven captions and `container()` writes the same eleven again thirty
|
||||
lines away; a twelfth row added to one and not the other silently truncates.
|
||||
|
||||
```go
|
||||
type detailRowSpec struct {
|
||||
caption string
|
||||
value fyne.CanvasObject
|
||||
}
|
||||
|
||||
func (d *detailsPanel) metadataRows() []detailRowSpec
|
||||
```
|
||||
|
||||
`container()` derives `capW := captionColumnWidth(captions(specs)...)`, then
|
||||
walks the slice two at a time into `detailRowPair`, with the odd tail
|
||||
(Statistics) falling through to a single `detailRow`. `detailCaptionWidth`
|
||||
disappears.
|
||||
3. `src/ui/settings_view.go:26-29,440-445` — delete `settingsLabelWidth` and
|
||||
derive the caption box the same way. Worth 21.7 px per column on top of
|
||||
Stage 1 (993.3 → 971.7), and it removes the last raw-pixel width in Settings.
|
||||
`settingsRow` gains a `captionWidth float32` first parameter and `settingsView`
|
||||
computes it once from the full caption list — exactly how `detailRow` already
|
||||
takes the width `container()` measured, rather than re-measuring per row.
|
||||
4. `src/ui/settings_view.go:299-311` — **F8**: delete both transparent
|
||||
`canvas.Rectangle` spacers for
|
||||
|
||||
```go
|
||||
container.New(
|
||||
layout.NewCustomPaddedLayout(2*theme.Padding(), 0, theme.Padding(), 0),
|
||||
container.NewHBox(saveSettings, cancelSettings, restoreDefaults, settingsStatus),
|
||||
)
|
||||
```
|
||||
|
||||
which reproduces the current 12 px top gap exactly while fixing the left
|
||||
inset to the 4 px the comment promises — measured, the rectangle version puts
|
||||
the first button at x = 8, because `HBox` adds its own padding *after* the
|
||||
spacer. The `image/color` and `fyne.io/fyne/v2/canvas` imports go with it.
|
||||
5. **N1** (not in the review) — `fyne.TextTruncate` as a `Wrapping` value is
|
||||
deprecated in Fyne 2.7.4 and commit 706aa8e converted exactly one call site.
|
||||
Eight remain: `settings_view.go:128,442`,
|
||||
`jobs_view_details.go:67,155,187,195`, `history_view.go:88,139`. Convert them
|
||||
to `Truncation = fyne.TextTruncateClip` in one pass. **`jobs_view_details.go:67`
|
||||
and `:155` must change together** — the second measures a sample label that
|
||||
has to stay identical to the first, which is the whole basis of
|
||||
`activityRowsHeight`.
|
||||
6. `src/ui/layout.go:121-136` — **F9 (nit half)**: `captionValueLayout.MinSize`
|
||||
and `Layout` both return silently when `len(objects) != 2`. Document why that
|
||||
is acceptable (package-private, one constructor) at the type, so the next
|
||||
reader does not have to work it out. The substantive half of F9 is Stage 8.
|
||||
|
||||
**Tests**
|
||||
|
||||
- `src/ui/jobs_view_test.go` — `TestDetailCaptionWidthCoversEveryCaption`: for
|
||||
every spec `metadataRows()` returns, the bold caption measures no wider than
|
||||
`captionColumnWidth` returns. This is the guard that makes the single list
|
||||
self-enforcing.
|
||||
- `src/ui/settings_view_test.go` — the same assertion for the settings captions.
|
||||
- `src/ui/layout_test.go` — `captionColumnWidth` over an empty list, one caption,
|
||||
and at two text sizes.
|
||||
|
||||
## Part B — larger, but decided
|
||||
|
||||
### Stage 7 — split `settings_view.go` (F13)
|
||||
|
||||
445 lines against the ~250 guideline in [ARCHITECTURE.md](ARCHITECTURE.md), and
|
||||
`jobs_view.go` already demonstrates the shape. Stages 1 and 6 remove roughly 25
|
||||
lines from it, so the split should follow them, not precede them.
|
||||
|
||||
**Changes**
|
||||
|
||||
- `settings_view.go` — `settingsView`: field construction, save, load, validate.
|
||||
- `settings_view_layout.go` — `settingsSection`, `settingsRow`, the two columns,
|
||||
the button row.
|
||||
- `settings_view_helpers.go` — `fyneVersion`, `mustParseURL`,
|
||||
`settingsFolderPath`, `openFolder`, `chooseFile`/`chooseFolder`.
|
||||
- Two composition inconsistencies to settle in the same pass rather than carry
|
||||
across the split:
|
||||
- *Application* and *About* use `settingsSection` (condensed); *Queue* and
|
||||
*Storage* inline `container.NewVBox(header, rows…)` (theme spacing). Two
|
||||
spellings of "a titled block of rows" in one function. Resolve to
|
||||
`settingsSection(title, spacing, rows…)` or two named constructors.
|
||||
- `chooseFile` and `chooseJSONFile` differ only by `SetFilter`. One function
|
||||
taking a filter (`nil` for none) removes the copy. `chooseFile` is also
|
||||
`job_dialog.go`'s, so it belongs with the helpers, not with Settings.
|
||||
- `docs/ARCHITECTURE.md` — add a file-structure table for `settings_view.go`
|
||||
beside the existing `jobs_view.go` one (~line 206).
|
||||
|
||||
**Tests:** no new behaviour, so the existing suite is the check. Worth adding
|
||||
`settings_view_test.go` coverage for the deduplicated picker's filter argument.
|
||||
|
||||
### Stage 8 — draggable master/detail split (F15, F9)
|
||||
|
||||
`jobs_view.go:365-366` pins the sidebar at its `MinSize` in a `Border` left slot,
|
||||
so the user can never trade list width for detail width. `container.NewHSplit`
|
||||
is the idiomatic Fyne answer: a draggable divider, `SetOffset` for the initial
|
||||
ratio. It gives F9 a user-controlled escape — the details value column bottoms
|
||||
out at 107.8 px today, kept non-empty only by the unrelated 460 px minimum on the
|
||||
command-output scroll.
|
||||
|
||||
**Decided: the divider position is not persisted.** No `Config` field, no
|
||||
`domain`/`storage`/`app` changes, no config-compatibility tests — the stage stays
|
||||
inside `src/ui` and is one commit. A restart reopens at the computed default.
|
||||
If persistence is wanted later it is an additive `omitempty` field whose zero
|
||||
value means "compute the default", which is exactly the pattern
|
||||
[STANDARDS.md](STANDARDS.md) already requires.
|
||||
|
||||
**Changes**
|
||||
|
||||
1. `src/ui/jobs_view.go:365-366` — replace the `Border` with
|
||||
`container.NewHSplit(sidebar, container.NewPadded(dp.container()))`.
|
||||
2. Initial offset: `SetOffset` takes a ratio, but the sidebar's natural width is
|
||||
absolute (448). A fixed ratio is wrong at both ends — 0.44 fits 1024 but hands
|
||||
the sidebar 700 px at 1600. Compute it at build time from
|
||||
`sidebar.MinSize().Width / defaultWindowWidth` (the const Stage 1 introduces),
|
||||
with a comment saying why it is derived rather than a literal.
|
||||
3. This retires what is left of F7's wrapper. Stage 3 still lands first and on
|
||||
its own — Part A has to stand up even if this stage were deferred — which
|
||||
costs one extra line in `jobsSidebar`: Stage 3 anchors it on
|
||||
`panel.Objects[1]` (the `Border` left slot), this stage moves it to the
|
||||
split's `Leading`.
|
||||
|
||||
**Tests**
|
||||
|
||||
- `src/ui/jobs_view_test.go` — `TestJobsSplitOpensAtTheSidebarWidth`: assert the
|
||||
computed offset gives the leading pane at least its content minimum at the
|
||||
default window width, so the toolbar is never born clipped.
|
||||
- Verify manually (Verification step 7) that dragging the divider hard left
|
||||
degrades the details pane instead of clipping it: `HSplit` lets either side
|
||||
shrink to its content minimum, which for the details pane is the 460 px
|
||||
command-output floor.
|
||||
|
||||
### Stage 9 — close-out
|
||||
|
||||
1. `docs/STANDARDS.md` — add the rule this review established, under *Code
|
||||
quality*: **a size that must follow the theme is measured at build time, not
|
||||
written as a pixel constant** — `activityRowsHeight`, `captionColumnWidth`,
|
||||
`rowOverlap` and `textColumnWidth` as the examples.
|
||||
2. `docs/ROADMAP.md` — remove the *GUI review — custom layouts and composition*
|
||||
item outright. With Part B in scope, nothing from it carries forward.
|
||||
3. `docs/CHANGELOG.md` — a `## 0.16.0` section. User-visible: the window opens
|
||||
at the size it is asked for and can be dragged smaller; the Jobs pane divider
|
||||
is draggable; History columns stay readable on a scaled UI; the Settings
|
||||
button row is aligned as intended; the details/settings blocks shift by ~2 px.
|
||||
The rest is internal and belongs in the commit messages, not here.
|
||||
4. `src/app/version.go` — `0.15.0` → `0.16.0`.
|
||||
5. Delete `docs/PLAN-gui-layout.md` and `docs/GUI-LAYOUT-REVIEW.md`, the way
|
||||
`docs/plans/per-job-timeout.md` was removed once shipped (2ab5f07) — the
|
||||
findings live on in the CHANGELOG and STANDARDS entries, and with the whole
|
||||
plan implemented the review has nothing left to hold open.
|
||||
|
||||
## Recommended model per stage
|
||||
|
||||
Which Claude model to run each stage on. The split is by *judgement density*,
|
||||
not by diff size: the stages that only move code around are the cheap ones, and
|
||||
the stages that can break something silently are not.
|
||||
|
||||
| Stage | Model | Why |
|
||||
|---|---|---|
|
||||
| 1 — window minimum (F2, F3) | Sonnet 5 | Small deletions, but the new geometry test has to assert the right thing. The plan already fixes the numbers, so there is little left to decide. |
|
||||
| 2 — stock layout, `rowOverlap` (F4, F5) | Sonnet 5 | Mechanical: delete a type, swap three call sites, add two imports. The equivalence it rests on is already measured. |
|
||||
| 3 — sidebar floor (F7) | Sonnet 5 | One constant and one wrapper out; the only care needed is the `NewBorder` slot ordering in the test helper, which the plan spells out. |
|
||||
| **4 — redraw cost (F11, F12)** | **Opus 5** | The one stage that can break History silently. Caching the sorted slice forces the length callback off `len(*events)`, and the `list.Refresh()` at `jobs_view.go:181` is load-bearing on an early-return path. Both are easy to get subtly wrong and neither shows up as a compile error. |
|
||||
| 5 — History columns (F6, F14) | Sonnet 5 | The helper's shape is specified. One cross-package check: the trigger and state sample strings must match what `app`/`runner` actually emit. |
|
||||
| 6 — caption width, button row, truncation (F8, F10, N1) | Sonnet 5 | Broad but mechanical. The one trap is paired: `jobs_view_details.go:67` and `:155` must change together or `activityRowsHeight` stops mirroring the real row. |
|
||||
| 7 — split `settings_view.go` (F13) | Sonnet 5 | Pure code movement plus two small dedups. Large diff, low judgement — but re-read the moved file ends for dropped functions. |
|
||||
| **8 — HSplit (F15, F9)** | **Opus 5** | Behaviour change. The offset derivation and "does the pane degrade or clip when dragged hard left" are judgement calls that a headless test cannot settle. |
|
||||
| **9 — close-out (docs, version)** | **Opus 5** | CHANGELOG and STANDARDS prose held to the standard the rest of `docs/` sets, which is the expensive part of this repo's doc convention. |
|
||||
|
||||
Notes:
|
||||
|
||||
- **Escalate on the second failure.** If a stage's tests fail twice for reasons
|
||||
the plan did not anticipate, the assumption behind that stage is wrong — move
|
||||
it to Opus 5 rather than iterating.
|
||||
- **Verification is model-independent.** `scripts\test.bat` and the manual GUI
|
||||
pass below are the gate regardless of who wrote the diff; a cheaper model does
|
||||
not mean a lighter check.
|
||||
- Haiku 4.5 is deliberately not recommended for any stage here: every one of
|
||||
them edits GUI code whose correctness is geometric rather than textual, and
|
||||
the cheapest stages are already short enough that the saving is small.
|
||||
- Fable 5 is left out because its trade-offs are not characterised well enough
|
||||
here to recommend it for a specific stage, not because it was judged unfit.
|
||||
|
||||
## Verification
|
||||
|
||||
Per stage:
|
||||
|
||||
```bash
|
||||
export PATH="/c/msys64/ucrt64/bin:$PATH"; export CGO_ENABLED=1; go build ./... && go vet ./... && go test -race ./...
|
||||
```
|
||||
|
||||
or `scripts\test.bat` from the cgo-enabled PowerShell shell described in
|
||||
[CLAUDE.md](../CLAUDE.md).
|
||||
|
||||
After Part A, run the app (`go run ./cmd/gosentry`) and check the things no
|
||||
headless test covers:
|
||||
|
||||
1. The window opens at 1024×660 and can be dragged **narrower** than it opens —
|
||||
the F1 symptom, gone.
|
||||
2. Settings looks unchanged at the default window width: controls still fill
|
||||
their column, captions still align, the config path is readable and clips
|
||||
only when the window is genuinely narrow.
|
||||
3. The Save/Cancel/Defaults row keeps its gap below the separator and sits 4 px
|
||||
from the left edge, not 8.
|
||||
4. Jobs rows and the details metadata block are unchanged in Detailed and in
|
||||
Compact; the Application/About blocks in Settings are 2 px tighter (Stage 2's
|
||||
one deliberate change).
|
||||
5. History still sorts both ways from the Time header, new runs still append,
|
||||
and columns hold their content.
|
||||
6. Repeat 2, 4 and 5 with a scaled desktop (or a temporary theme override with a
|
||||
larger `SizeNameText`): nothing that used to be a pixel constant should clip.
|
||||
7. Resize the window to its minimum in both directions and confirm the details
|
||||
pane degrades rather than clipping — the check the roadmap item asked for.
|
||||
@@ -81,50 +81,6 @@ Design notes / open questions:
|
||||
Service exposes import/export operations; the UI only picks the file and
|
||||
shows the outcome.
|
||||
|
||||
### GUI review — custom layouts and composition
|
||||
|
||||
**The review has been carried out — its findings are in
|
||||
[GUI-LAYOUT-REVIEW.md](GUI-LAYOUT-REVIEW.md).** This item stays open until they
|
||||
are applied; F9, F13 and F15 there are larger than a single fix and come back
|
||||
here once the rest has landed. The agenda below is what the pass was asked to
|
||||
answer.
|
||||
|
||||
The `ui` package has accumulated hand-written layouts and tuned constants that
|
||||
work but have never been reviewed as a whole:
|
||||
[`layout.go`](../src/ui/layout.go) holds four custom `fyne.Layout`
|
||||
implementations (`minWidthLayout`, `compactVBoxLayout`, `fixedHeightLayout`,
|
||||
`captionValueLayout`), and the views drive them with negative spacings
|
||||
(`detailRowSpacing = -8`, `jobRowSpacing = -8`, `settingsRowSpacing = -6`) that
|
||||
cancel out the built-in padding of Fyne widgets.
|
||||
|
||||
Do a focused pass over composition only — not a general code review — and
|
||||
answer, per layout and per constant: is it still needed, is it the smallest
|
||||
thing that works, and does it hold up at different window sizes and theme
|
||||
scales.
|
||||
|
||||
What to look for:
|
||||
|
||||
- *Negative spacing as a workaround.* Pulling rows together to overlap label
|
||||
padding is a workaround for widget metrics, not a layout decision. Check
|
||||
whether a `widget.Form`, a grid, or a custom text row would express the same
|
||||
result without depending on the padding a future Fyne release may change.
|
||||
- *Hard-coded pixel constants.* Widths and heights expressed in raw pixels
|
||||
(`logColumnMinWidth`, `minJobsSidebarWidth`, `settingsControlWidth`) do not
|
||||
follow `theme.Padding()` / text size, so they behave differently under a
|
||||
scaled UI. `activityRowsHeight` already derives its height from the theme —
|
||||
decide which of the rest should do the same.
|
||||
- *Layouts with one call site.* `fixedHeightLayout` is used once. If a stock
|
||||
container expresses the same intent, deleting the type is a net win — the
|
||||
complexity rule in [REVIEW.md](REVIEW.md) §2 applies to layouts too.
|
||||
- *File size.* `settings_view.go` is well past the ~250-line guideline in
|
||||
[ARCHITECTURE.md](ARCHITECTURE.md) and should be split the way `jobs_view.go`
|
||||
was.
|
||||
- *Behaviour at small sizes.* The details pane was condensed to fit 720p; verify
|
||||
the current composition still degrades sensibly when the window is narrow or
|
||||
short, instead of clipping.
|
||||
|
||||
Findings that are single fixes go straight in; anything larger comes back here.
|
||||
|
||||
### Window size persistence *(frozen)*
|
||||
|
||||
Window size is currently **not** saved on quit or close. Saving was disabled
|
||||
|
||||
+13
-2
@@ -12,6 +12,16 @@ what a whole-project review looks at, in [REVIEW.md](REVIEW.md).
|
||||
- Fixes with severity ≥ medium → regression test.
|
||||
- Documented intentional behavior → section below, not a backlog bug.
|
||||
- UI view constructors accept `*app.Service`; call `app.Open()` only from `run.go`.
|
||||
- A size that must follow the theme is **measured at build time, not written as
|
||||
a pixel constant.** `theme.Padding()` and text metrics depend on the running
|
||||
app's theme, text size, and DPI, so a hand-tuned number is only correct for
|
||||
the one theme it was tuned against and clips under any other. Measure the real
|
||||
widget, or derive the value from the theme, in a named helper: `rowOverlap`
|
||||
(theme padding), `captionColumnWidth` and `textColumnWidth` (the widest of the
|
||||
actual strings), `activityRowsHeight` (the list's own row template). The same
|
||||
applies to a ratio computed from an absolute width — see `initialSplitOffset`.
|
||||
A raw pixel literal is left only where nothing about it tracks the theme, and
|
||||
says so in a comment.
|
||||
|
||||
## Config file compatibility
|
||||
|
||||
@@ -59,5 +69,6 @@ change to their shape has to stay compatible on its own.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Larger or blocked work is tracked in [ROADMAP.md](ROADMAP.md) (window size
|
||||
persistence, History column filters, CI coverage gate).
|
||||
Larger or blocked work is tracked in [ROADMAP.md](ROADMAP.md) (update check from
|
||||
GitHub releases, cron-table import/export, window size persistence, History
|
||||
column filters).
|
||||
|
||||
+162
-54
@@ -22,6 +22,10 @@ Both scripts run:
|
||||
1. `go vet ./...` — static analysis for common errors and suspicious code patterns
|
||||
2. `go test -race ./...` — tests with race condition detection enabled
|
||||
|
||||
The GUI tests build the Fyne desktop backend, so CGO must be enabled; on Windows
|
||||
that means the MSYS2 UCRT64 toolchain described in
|
||||
[DEVELOPMENT.md](DEVELOPMENT.md).
|
||||
|
||||
### Manual test commands
|
||||
|
||||
Run all tests:
|
||||
@@ -74,6 +78,20 @@ Tests schedule parsing and validation.
|
||||
|
||||
---
|
||||
|
||||
### src/domain/config_test.go
|
||||
|
||||
**Package:** `domain`
|
||||
|
||||
Tests the normalization rule shared by every consumer of the jobs-list density
|
||||
setting.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestJobListViewIsCompact` | Verifies only the exact `"compact"` value selects one-line rows: empty, differently-cased, and unrecognised values all read as detailed. |
|
||||
| `TestDefaultConfigUsesDetailedJobList` | Verifies `DefaultConfig` selects the detailed job list. |
|
||||
|
||||
---
|
||||
|
||||
### src/app/service_test.go
|
||||
|
||||
**Package:** `app`
|
||||
@@ -114,13 +132,15 @@ Tests all mutating operations on the Service, scheduler integration, and setting
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestSetGlobalPauseUpdatesRuntimesAndEmits` | Verifies that `SetGlobalPause` updates all job runtimes, emits `SchedulerStateChanged`, and persists state. |
|
||||
| `TestSetGlobalPausePersistsToConfigFile` | Verifies the paused flag reaches `gosentry.json`, which is what makes the pause survive a restart. |
|
||||
| `TestServiceRebuiltFromPausedStoreStartsPaused` | Verifies a Service built from a paused config starts paused, with the paused next-run text applied before the first tick. |
|
||||
| `TestRunNowUsesRunnerAndRecords` | Verifies that `RunNow` invokes the runner, records a `RunRecord`, and emits `RunRecorded`. |
|
||||
| `TestRunNowNotFound` | Verifies that `RunNow` returns an error for an unknown job ID. |
|
||||
| `TestRunNowRefusedWhileAlreadyRunning` | Verifies that a second concurrent `RunNow` on the same job is rejected while the first is in progress. |
|
||||
| `TestRunNowAllowedWhilePaused` | Verifies that `RunNow` is allowed when the global pause flag is set (pause stops scheduled runs only). |
|
||||
| `TestRunDueStartsDueJob` | Verifies that `RunDue` launches a job whose next-run time has passed. |
|
||||
| `TestRunDueSkipsJobNotYetDue` | Verifies that `RunDue` does not launch a job that is not yet due. |
|
||||
| `TestRunDueSkipsJobInRunningState` | Verifies that `RunDue` does not start a second concurrent run for an already-running job. |
|
||||
| `TestRunDueSkipsJobInRunningState` | Verifies that `RunDue` does not start a second concurrent run for an already-running job, even with a stale `NextDue` in the past. |
|
||||
| `TestRunDueDoesNothingWhilePaused` | Verifies that `RunDue` launches nothing when the global pause flag is set. |
|
||||
| `TestStartDrivesRunDueOnTick` | Verifies that `Service.Start` wires `RunDue` to the scheduler tick and that each tick advances state. |
|
||||
|
||||
@@ -135,6 +155,8 @@ Tests all mutating operations on the Service, scheduler integration, and setting
|
||||
| `TestUpdateSettingsAdoptsExistingJobsFile` | Verifies that selecting a jobs file that already exists replaces the job list with its contents, rebuilds runtimes, and emits `JobsLoaded`. |
|
||||
| `TestUpdateSettingsKeepsJobsWhenTheNewFileIsMissing` | Verifies that a path with no file behind it receives the current jobs instead (the rename/relocate case). |
|
||||
| `TestUpdateSettingsRefusesJobsFileSwitchWhileRunning` | Verifies that switching the jobs file is refused (and not persisted) while a job runs, while unrelated settings still save. |
|
||||
| `TestSetJobListViewPersistsToConfigFile` | Verifies the Jobs-list density preference reaches `gosentry.json`, so the chosen view reopens after a restart. |
|
||||
| `TestSetJobListViewNormalizesUnknownValue` | Verifies anything but `"compact"` is stored as `"detailed"`, so the config never gains a value no reader understands. |
|
||||
| `TestPrependLogCapsActivityList` | Verifies that the activity log never grows beyond its maximum cap. |
|
||||
|
||||
---
|
||||
@@ -143,7 +165,8 @@ Tests all mutating operations on the Service, scheduler integration, and setting
|
||||
|
||||
**Package:** `app`
|
||||
|
||||
Tests overlap policy, sequential execution, run statistics, and scheduler edge cases using injected `runJob` and `primeDue`.
|
||||
Tests overlap policy, sequential execution, run statistics, timeout resolution,
|
||||
and scheduler edge cases using injected `runJob` and `primeDue`.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
@@ -160,6 +183,7 @@ Tests overlap policy, sequential execution, run statistics, and scheduler edge c
|
||||
| `TestRunNowSequentialGuard` | Manual run refused while another job runs in sequential mode. |
|
||||
| `TestStartRunLockedRollbackOnSaveFailure` | Regression: run does not start when `SaveJobs` fails. |
|
||||
| `TestRunDueQueueDrainSkippedWhenPaused` | Queued overlaps are not drained while the scheduler is paused. |
|
||||
| `TestEffectiveTimeout` | Verifies the three-state resolution: `nil` inherits the global default, a positive value overrides it, and an explicit `0` means no timeout without inheriting. |
|
||||
|
||||
---
|
||||
|
||||
@@ -187,6 +211,7 @@ Tests display-formatting helpers used by the UI.
|
||||
|------|---------|
|
||||
| `TestStatusText` | Verifies that job status codes map to the correct display strings. |
|
||||
| `TestEventText` | Verifies trigger-type labels for scheduled, manual, and UI triggers. |
|
||||
| `TestEventLine` | Verifies the one-line activity rendering of a `RunRecord`, including the log basename and the `Unknown` fallback for a blank trigger. |
|
||||
| `TestDisplayFolder` | Verifies that an empty folder string shows "No folder". |
|
||||
| `TestDisplayArguments` | Verifies that an empty arguments string shows "None". |
|
||||
| `TestDisplayRunMode` | Verifies run-mode labels for normal and start-only modes. |
|
||||
@@ -194,6 +219,7 @@ Tests display-formatting helpers used by the UI.
|
||||
| `TestDisplayIndex` | Verifies the list position of a job index in a filtered index slice. |
|
||||
| `TestDisplayStats` | Verifies statistics line formatting for the details panel. |
|
||||
| `TestDisplayOverlapPolicy` | Verifies per-job vs inherited global overlap policy labels. |
|
||||
| `TestDisplayTimeout` | Verifies the three timeout states read differently in the details panel: `45 s`, `no timeout`, and `… (global default)`. |
|
||||
|
||||
---
|
||||
|
||||
@@ -201,7 +227,7 @@ Tests display-formatting helpers used by the UI.
|
||||
|
||||
**Package:** `storage`
|
||||
|
||||
Tests JSON round-tripping and default generation.
|
||||
Tests JSON round-tripping, default generation, and backward compatibility.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
@@ -209,9 +235,11 @@ Tests JSON round-tripping and default generation.
|
||||
| `TestConfigRoundTrip` | Verifies that settings saved to JSON are reloaded with identical field values. |
|
||||
| `TestNormalizeJobsFillsDefaults` | Verifies that `normalizeJobs` assigns sequential IDs and sets default name, schedule, and command for jobs missing those fields. |
|
||||
| `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun` | Verifies that a missing config file is created with sane defaults and a sample job. |
|
||||
| `TestLoadOrCreateConfigKeepsZeroTimeoutOnReload` | Verifies that `default_timeout_seconds: 0` survives a reload rather than being normalized away — 0 is a value, not a missing field. |
|
||||
| `TestLoadOrCreateConfigMigratesJobsDir` | Verifies that a pre-0.15 `jobs_dir` becomes `jobs_file` pointing at the same `jobs.json`, and that the retired key is not written back. |
|
||||
| `TestLoadJobsFileReportsMissingWithoutCreating` | Verifies that `LoadJobsFile` reports a missing file as not-found without creating or seeding it, and normalizes the jobs it does load. |
|
||||
| `TestApplyConfigPathsDerivesJobsDir` | Verifies that the configured jobs file resolves against the program folder and that `Paths.JobsDir` is derived from it. |
|
||||
| `TestJobTimeoutRoundTripsThreeStates` | Verifies the on-disk encoding that keeps "inherit" and "no timeout" distinguishable: `nil` is omitted entirely, an explicit `0` is written and read back as set. |
|
||||
| `TestJobsJSONDoesNotPersistRuntimeNoise` | Verifies that `jobs.json` does not persist runtime state (LastRun, NextRun, etc.). Only durable job fields are stored. |
|
||||
|
||||
---
|
||||
@@ -233,7 +261,7 @@ Tests the timing-loop contract using a fake clock.
|
||||
|
||||
**Package:** `runner`
|
||||
|
||||
Tests command execution, exit code handling, output capture, and Windows-specific process behavior.
|
||||
Tests command execution, exit code handling, output capture, and the run timeout.
|
||||
|
||||
#### Log file tests
|
||||
|
||||
@@ -266,6 +294,14 @@ Tests command execution, exit code handling, output capture, and Windows-specifi
|
||||
|------|---------|
|
||||
| `TestRunJobFailsOnNonZeroExitCode` | Verifies that a nonzero process exit code results in "Failed" status with an "exit code N" detail. |
|
||||
|
||||
#### Timeout
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestRunJobTimesOut` | Verifies that a positive timeout kills a long-running command and reports `Timed out after <timeout>`. |
|
||||
| `TestRunJobZeroTimeoutMeansNoTimeout` | Verifies that a non-positive duration runs without a deadline, bounded only by the caller's context. |
|
||||
| `TestRunJobStartOnlyIgnoresTimeout` | Verifies that fire-and-forget jobs run on the untimed context, so the timeout never kills a process the runner is not waiting for. |
|
||||
|
||||
#### Start-only mode
|
||||
|
||||
| Test | Purpose |
|
||||
@@ -273,14 +309,39 @@ Tests command execution, exit code handling, output capture, and Windows-specifi
|
||||
| `TestRunJobStartOnlyDoesNotWaitForExitCode` | Verifies that `StartOnly: true` jobs launch and return "OK" immediately without waiting for the process to exit. |
|
||||
| `TestRunJobStartOnlyReportsStartFailure` | Verifies that `StartOnly: true` jobs still report "Failed" if the process cannot be started. |
|
||||
|
||||
#### Utility / Windows invocation
|
||||
---
|
||||
|
||||
| Test | Platform | Purpose |
|
||||
|------|----------|---------|
|
||||
| `TestDirectCommandDoesNotHideWindow` | Windows | Verifies that direct executable commands do not request hidden-window startup. |
|
||||
| `TestShellCommandHidesWindow` | Windows | Verifies that shell commands request hidden-window startup to prevent console flash. |
|
||||
| `TestShellCommandUsesWindowsSafeQuoting` | Windows | Verifies `cmd.exe /S /C` quoting for paths with spaces and special characters. |
|
||||
| `TestWindowsShellCommandLineQuotesUnquotedProgramPath` | Windows | Verifies that unquoted program paths in shell commands are quoted while preserving already-quoted arguments. |
|
||||
### src/runner/runner_windows_test.go
|
||||
|
||||
**Location:** `src/runner/runner_windows_test.go`
|
||||
**Build Tags:** `//go:build windows`
|
||||
|
||||
Tests the Windows shell invocation and hidden-window flags.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestDirectCommandDoesNotHideWindow` | Verifies that direct executable commands do not request hidden-window startup. |
|
||||
| `TestShellCommandHidesWindow` | Verifies that shell commands request hidden-window startup to prevent console flash. |
|
||||
| `TestShellCommandUsesWindowsSafeQuoting` | Verifies `cmd.exe /S /C` quoting for paths with spaces and special characters. |
|
||||
| `TestWindowsShellCommandLineQuotesUnquotedProgramPath` | Verifies that unquoted program paths in shell commands are quoted while preserving already-quoted arguments. |
|
||||
|
||||
---
|
||||
|
||||
### src/runner/seed_test.go
|
||||
|
||||
**Package:** `runner`
|
||||
|
||||
Tests `SeedStats`, which rebuilds aggregate run statistics from the `.log` files
|
||||
on disk at startup.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestSeedStatsBasic` | Verifies run/fail counts and the last, average, and maximum durations parsed from a job's log headers. |
|
||||
| `TestSeedStatsDurationLessLegacyLog` | Verifies a log written before the `duration` header still counts as a run but is excluded from the duration aggregates, so a missing duration cannot masquerade as a 0 ms run. |
|
||||
| `TestSeedStatsMaxFilesHonoured` | Verifies that only the newest `MaxLogFiles` logs are parsed when the limit is positive. |
|
||||
| `TestSeedStatsMissingDir` | Verifies a missing logs directory yields an empty map rather than an error or a panic. |
|
||||
| `TestSeedStatsUnknownJobProducesNoEntry` | Verifies log files that match no known job are ignored. |
|
||||
| `TestSeedStatsMatchesByJobID` | Verifies logs are matched by the `job_id` header even when two job names sanitize to the same filename. |
|
||||
|
||||
---
|
||||
|
||||
@@ -311,7 +372,6 @@ Tests Windows autostart via shortcuts in the Startup folder.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestParseRegistryRunValue` | Verifies that legacy `HKCU\...\Run` entry values are parsed correctly from `reg query` output (for migration/cleanup). |
|
||||
| `TestSameWindowsPathIgnoresCaseAndQuotes` | Verifies that Windows path comparison is case-insensitive and handles quote marks correctly. |
|
||||
| `TestSameWindowsPathHandlesSpaces` | Verifies that Windows path comparison matches paths with and without surrounding quotes. |
|
||||
| `TestSameWindowsPathStripsExtendedLengthPrefix` | Verifies that `\\?\`-prefixed paths are compared correctly after stripping the prefix. |
|
||||
@@ -332,7 +392,6 @@ Tests Linux autostart via XDG Desktop Entry files.
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestLinuxAutostartStartsInTray` | Verifies that the XDG Desktop Entry is created with `--start-in-tray` in the `Exec=` field. |
|
||||
| `TestLinuxAutostartRemovesLegacyDesktopEntry` | Verifies that enabling autostart also removes legacy PySentry service files left by earlier builds. |
|
||||
|
||||
---
|
||||
|
||||
@@ -350,43 +409,6 @@ Tests Linux desktop integration (`.desktop` file and icon under XDG data home).
|
||||
|
||||
---
|
||||
|
||||
### src/ui/jobs_view_test.go
|
||||
|
||||
**Package:** `ui`
|
||||
|
||||
Tests pure helper functions in the jobs view (no Fyne widget construction).
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestFilterValue` | Verifies that `filterValue` returns the correct display string for the current folder filter. |
|
||||
| `TestFolderOptionsAlwaysIncludesSentinels` | Verifies that the folder filter list always starts with "All" and "No folder" sentinel entries. |
|
||||
| `TestFolderOptionsAppendsUniqueFolders` | Verifies that folder names from the job list are appended once each, in order, without duplicates. |
|
||||
| `TestFilteredJobIndexesAll` | Verifies that the "All" filter returns indexes for every job. |
|
||||
| `TestFilteredJobIndexesByNamedFolder` | Verifies that filtering by a named folder returns only jobs in that folder. |
|
||||
| `TestFilteredJobIndexesNoFolder` | Verifies that the "No folder" filter returns only jobs with an empty folder field. |
|
||||
| `TestFilteredJobIndexesEmptySlice` | Verifies that filtering an empty job slice returns an empty index list. |
|
||||
| `TestLastJobLogsCapsAndCopies` | Verifies activity panel cap and defensive copy semantics. |
|
||||
| `TestLastJobLogsEmpty` | Verifies nil/empty log input returns an empty slice. |
|
||||
| `TestIndexOfID` | Verifies job lookup by ID returns `-1` when not found. |
|
||||
|
||||
---
|
||||
|
||||
### src/ui/history_view_test.go
|
||||
|
||||
**Package:** `ui`
|
||||
|
||||
Tests pure History tab helpers (no Fyne widget construction).
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestCollectActivityMergesAndSorts` | Verifies per-job logs are merged and sorted by time. |
|
||||
| `TestCollectActivitySkipsMissingRuntimes` | Verifies missing runtime entries are skipped safely. |
|
||||
| `TestHistoryCellText` | Verifies table cell text for all columns; empty trigger → `Unknown`. |
|
||||
| `TestLogFileName` | Verifies log path basename extraction on Windows and Unix paths. |
|
||||
| `TestNewEventUsesConsistentTimestampShape` | Verifies UI events use the same timestamp layout as run records. |
|
||||
|
||||
---
|
||||
|
||||
### src/platform/filemanager/filemanager_test.go
|
||||
|
||||
**Package:** `filemanager`
|
||||
@@ -402,15 +424,98 @@ success path is not tested: it would open a real file manager window.
|
||||
|
||||
---
|
||||
|
||||
### src/ui/jobs_view_test.go
|
||||
|
||||
**Package:** `ui`
|
||||
|
||||
Tests the Jobs tab: pure filter helpers, and — through Fyne's headless
|
||||
`test.NewApp()` — the geometry and redraw behaviour that only shows up once the
|
||||
widgets are assembled.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestFilterValue` | Verifies that `filterValue` returns the correct display string for the current folder filter. |
|
||||
| `TestFolderOptionsAlwaysIncludesSentinels` | Verifies that the folder filter list always starts with "All" and "No folder" sentinel entries. |
|
||||
| `TestFolderOptionsAppendsUniqueFolders` | Verifies that folder names from the job list are appended once each, in order, without duplicates. |
|
||||
| `TestFilteredJobIndexesAll` | Verifies that the "All" filter returns indexes for every job. |
|
||||
| `TestFilteredJobIndexesByNamedFolder` | Verifies that filtering by a named folder returns only jobs in that folder. |
|
||||
| `TestFilteredJobIndexesNoFolder` | Verifies that the "No folder" filter returns only jobs with an empty folder field. |
|
||||
| `TestFilteredJobIndexesEmptySlice` | Verifies that filtering an empty job slice returns an empty index list. |
|
||||
| `TestNextJobListViewFlipsBothWays` | Verifies the density toggle alternates between detailed and compact from either starting value. |
|
||||
| `TestViewToggleTextNamesTheAction` | Verifies the toggle button is labelled with the action it performs, not the state it is in. |
|
||||
| `TestJobListViewToggleShrinksRowsAndPersists` | End-to-end: one tap shrinks the row height, relabels the button, and reaches the config; tapping back undoes all three. |
|
||||
| `TestJobListViewCompactConfigOpensCompact` | Verifies the persisted density is honoured at build time, not only after a tap. |
|
||||
| `TestJobsSidebarWidthIsItsContent` | Regression guard: nothing but the sidebar's own toolbar row imposes a width floor on it. |
|
||||
| `TestJobsSplitOpensAtTheSidebarWidth` | Verifies the derived split offset opens the divider at the sidebar's own width at the default window size — enough that the toolbar is never born clipped, and no more. |
|
||||
| `TestToolbarButtonRedrawsRowAndDetails` | Regression guard: with the duplicate refreshes removed from the handlers, `refreshView` alone must re-snapshot the jobs and repopulate the details pane. |
|
||||
| `TestDetailCaptionWidthCoversEveryCaption` | Verifies every caption `metadataRows` returns fits the measured caption column, which is what makes the single row list self-enforcing. |
|
||||
|
||||
---
|
||||
|
||||
### src/ui/history_view_test.go
|
||||
|
||||
**Package:** `ui`
|
||||
|
||||
Tests the History tab: the pure activity helpers and the sorted-snapshot and
|
||||
column-width behaviour of the assembled table.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestCollectActivityMergesAndSorts` | Verifies per-job logs are merged and sorted by time. |
|
||||
| `TestCollectActivitySkipsMissingRuntimes` | Verifies missing runtime entries are skipped safely. |
|
||||
| `TestHistoryCellText` | Verifies table cell text for all columns; empty trigger → `Unknown`. |
|
||||
| `TestLogFileName` | Verifies log path basename extraction on Windows and Unix paths. |
|
||||
| `TestNewEventUsesConsistentTimestampShape` | Verifies UI events use the same timestamp layout as run records. |
|
||||
| `TestLastJobLogsCapsAndCopies` | Verifies activity panel cap and defensive copy semantics. |
|
||||
| `TestLastJobLogsEmpty` | Verifies nil/empty log input returns an empty slice. |
|
||||
| `TestIndexOfID` | Verifies job lookup by ID returns `-1` when not found. |
|
||||
| `TestHistorySortToggleKeepsRowsInSync` | Regression guard for the cached sorted snapshot: the length callback and the cells must be refilled together, or the row count and the cell contents disagree. |
|
||||
| `TestHistoryCellTemplateIsPlainText` | Verifies the cell template already carries the zero `TextStyle`, since the per-cell assignment that used to reset it is gone. |
|
||||
| `TestTextColumnWidthClamps` | Covers the three shapes of `textColumnWidth`: below the minimum, in range, and capped at the maximum. |
|
||||
| `TestHistoryColumnsFitTheirContent` | Verifies every column is at least as wide as its widest known or present value, at the default text size and at a scaled theme. |
|
||||
|
||||
---
|
||||
|
||||
### src/ui/settings_view_test.go
|
||||
|
||||
**Package:** `ui`
|
||||
|
||||
Tests pure Settings tab helpers (no Fyne widget construction).
|
||||
Tests the Settings tab helpers and the row layout.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestSettingsFolderPath` | Verifies the folder the Logs directory "Open" button targets: blank text yields no path, a relative path resolves against the application directory, an absolute path is used as typed. |
|
||||
| `TestSettingsRowStretchesItsControl` | Verifies the row's centre slot already stretches the control to the column width — the property that made a fixed-width wrapper around it redundant. |
|
||||
| `TestChooseFileAppliesFilter` | Verifies the deduplicated picker opens a dialog both with a nil filter (the command browser) and with a concrete one (`chooseJSONFile`). |
|
||||
| `TestSettingsCaptionsCoverEveryRow` | Verifies every caption used in a row is present in `settingsCaptions` and fits the measured caption column. |
|
||||
|
||||
---
|
||||
|
||||
### src/ui/layout_test.go
|
||||
|
||||
**Package:** `ui`
|
||||
|
||||
Tests the theme-derived sizing helpers in `layout.go`.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestRowOverlapMatchesInnerPadding` | Pins `rowOverlap` to `-theme.InnerPadding()` under two themes, the property that lets it follow a theme instead of drifting from a hand-tuned literal. |
|
||||
| `TestCaptionColumnWidth` | Covers no captions, one, and several of varying length, at two text sizes. |
|
||||
|
||||
---
|
||||
|
||||
### src/ui/theme_test.go
|
||||
|
||||
**Package:** `ui`
|
||||
|
||||
Tests the branded theme and the stored theme choice.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestGoSentryThemeBrandColors` | Verifies the brand colors land on the semantically correct `ColorName`s in both the light and dark variants. |
|
||||
| `TestGoSentryThemeDelegatesUnbrandedColors` | Verifies unbranded color names fall through to the base theme rather than rendering transparent. |
|
||||
| `TestThemeForChoice` | Verifies the GoSentry choice yields the branded primary and every other value — including the empty legacy one — yields the default theme. |
|
||||
| `TestThemeLabelRoundTrip` | Verifies the dropdown labels round-trip and that the empty value maps to the Default label rather than a blank option. |
|
||||
|
||||
---
|
||||
|
||||
@@ -418,10 +523,11 @@ Tests pure Settings tab helpers (no Fyne widget construction).
|
||||
|
||||
**Package:** `ui`
|
||||
|
||||
Smoke test for main view construction with an injected `*app.Service`.
|
||||
Tests main view construction with an injected `*app.Service`.
|
||||
|
||||
| Test | Purpose |
|
||||
|------|---------|
|
||||
| `TestMainViewFitsTheDefaultWindowSize` | Verifies the assembled content's minimum fits the window size the app asks for, so Fyne never silently widens the window past it. The store's config path is deliberately long, since it was the path label that used to grow the Settings tab. |
|
||||
| `TestMainViewBuilds` | Verifies `newMainView` assembles tabs without panic using `fyne.io/fyne/v2/test`. |
|
||||
|
||||
---
|
||||
@@ -442,10 +548,12 @@ Smoke test for main view construction with an injected `*app.Service`.
|
||||
|
||||
7. **Regression on serious fixes** — Any fix from an internal review with severity ≥ medium gets a targeted regression test (see `run_test.go` for examples).
|
||||
|
||||
8. **Geometry is measured, not eyeballed** — The `ui` tests that build widgets under `test.NewApp()` assert sizes and offsets, and several re-run under a scaled theme. That is what keeps [STANDARDS.md](STANDARDS.md)'s "measure at build time, never a pixel constant" rule enforceable rather than aspirational.
|
||||
|
||||
---
|
||||
|
||||
## Remaining Test Coverage Gaps
|
||||
|
||||
- Full GUI E2E — tab navigation, dialog flows, and native file pickers are not exercised end-to-end
|
||||
- Full GUI E2E — tab navigation, dialog flows, and native file pickers are not exercised end-to-end; the `ui` tests assemble views and measure them, but nothing drives a real window.
|
||||
- History is session-only by design — `.log` files seed aggregate stats only, not the History table (see [STANDARDS.md](STANDARDS.md))
|
||||
- `layout.go` custom layouts — optional Fyne `test.NewApp()` coverage when CGO is available in CI
|
||||
- Fyne's headless driver cannot report a maximized window, which is why window-size persistence stays frozen in [ROADMAP.md](ROADMAP.md)
|
||||
|
||||
+1
-1
@@ -3,4 +3,4 @@ package app
|
||||
// Version is the application version shown in the GUI and used by build
|
||||
// scripts in artifact names. It is a var rather than a const so release builds
|
||||
// can override it with Go ldflags when CI tags a build.
|
||||
var Version = "0.15.0"
|
||||
var Version = "1.0.0"
|
||||
|
||||
+6
-1
@@ -345,6 +345,11 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) {
|
||||
sidebarHeader := container.NewVBox(globalControls, widget.NewSeparator(), filterRow, toolbar)
|
||||
sidebar := container.NewBorder(sidebarHeader, nil, nil, nil, list)
|
||||
|
||||
panel := container.NewBorder(nil, nil, sidebar, nil, container.NewPadded(dp.container()))
|
||||
// A split rather than a Border left slot: the border pinned the sidebar at its
|
||||
// MinSize forever, so the user could never trade list width for detail width.
|
||||
// The divider lets either pane grow, and neither can be dragged below its own
|
||||
// content minimum.
|
||||
panel := container.NewHSplit(sidebar, container.NewPadded(dp.container()))
|
||||
panel.SetOffset(initialSplitOffset(sidebar.MinSize().Width))
|
||||
return panel, refreshView
|
||||
}
|
||||
|
||||
@@ -60,6 +60,12 @@ func newDetailsPanel(firstJob job, rt *domain.JobRuntime, globalOverlapPolicy do
|
||||
// The height here is only a floor: the scroll grows to fill whatever space the
|
||||
// border layout gives it, so keep the minimum small so the whole window can be
|
||||
// shrunk on short (720p) screens. Long output stays reachable by scrolling.
|
||||
// The width, unlike the height, is load-bearing outside this widget: it is the
|
||||
// details pane's widest minimum, so it is also what keeps the metadata value
|
||||
// column non-empty — captionValueLayout hands the value whatever is left after
|
||||
// the caption and has no floor of its own (see its comment in layout.go).
|
||||
// Lowering it narrows those values with nothing to warn about it; the user can
|
||||
// only widen them, by dragging the jobs split divider left.
|
||||
d.commandOutputScroll.SetMinSize(fyne.NewSize(460, 70))
|
||||
d.logs = widget.NewList(
|
||||
func() int { return len(d.selectedLogs) },
|
||||
|
||||
+50
-16
@@ -7,6 +7,7 @@ import (
|
||||
"gitea.mixdep.ru/mix/gosentry/src/domain"
|
||||
|
||||
"fyne.io/fyne/v2"
|
||||
"fyne.io/fyne/v2/container"
|
||||
"fyne.io/fyne/v2/test"
|
||||
"fyne.io/fyne/v2/widget"
|
||||
)
|
||||
@@ -135,19 +136,24 @@ func findFirst(root fyne.CanvasObject, match func(fyne.CanvasObject) bool) fyne.
|
||||
return nil
|
||||
}
|
||||
|
||||
// jobsSplit returns the view's master/detail split. newJobsView assembles the
|
||||
// panel as container.NewHSplit(sidebar, details), so the two panes are reached
|
||||
// through Leading and Trailing.
|
||||
func jobsSplit(t *testing.T, content fyne.CanvasObject) *container.Split {
|
||||
t.Helper()
|
||||
split, ok := content.(*container.Split)
|
||||
if !ok {
|
||||
t.Fatal("jobs view is not the expected Split container")
|
||||
}
|
||||
return split
|
||||
}
|
||||
|
||||
// jobsSidebar narrows the search to the left pane. The details panel has a
|
||||
// widget.List of its own (the activity log), so a search from the whole view
|
||||
// would find the wrong one. newJobsView assembles the panel as
|
||||
// container.NewBorder(nil, nil, sidebar, nil, ...); NewBorder keeps the centre
|
||||
// object first and appends the border slots after it, so panel.Objects[1] is
|
||||
// the left (sidebar) slot.
|
||||
// would find the wrong one.
|
||||
func jobsSidebar(t *testing.T, content fyne.CanvasObject) fyne.CanvasObject {
|
||||
t.Helper()
|
||||
panel, ok := content.(*fyne.Container)
|
||||
if !ok || len(panel.Objects) < 2 {
|
||||
t.Fatal("jobs view is not the expected Border container")
|
||||
}
|
||||
return panel.Objects[1]
|
||||
return jobsSplit(t, content).Leading
|
||||
}
|
||||
|
||||
func jobsList(t *testing.T, content fyne.CanvasObject) *widget.List {
|
||||
@@ -193,15 +199,10 @@ func jobsToolbarButton(t *testing.T, content fyne.CanvasObject, text string) *wi
|
||||
return found.(*widget.Button)
|
||||
}
|
||||
|
||||
// jobsDetails narrows the search to the right pane. NewBorder keeps the centre
|
||||
// object first, so panel.Objects[0] is the details pane (see jobsSidebar).
|
||||
// jobsDetails narrows the search to the right pane (see jobsSidebar).
|
||||
func jobsDetails(t *testing.T, content fyne.CanvasObject) fyne.CanvasObject {
|
||||
t.Helper()
|
||||
panel, ok := content.(*fyne.Container)
|
||||
if !ok || len(panel.Objects) == 0 {
|
||||
t.Fatal("jobs view is not the expected Border container")
|
||||
}
|
||||
return panel.Objects[0]
|
||||
return jobsSplit(t, content).Trailing
|
||||
}
|
||||
|
||||
// jobsDetailsActivity returns the "Selected job activity" list, the only
|
||||
@@ -347,6 +348,39 @@ func TestJobsSidebarWidthIsItsContent(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestJobsSplitOpensAtTheSidebarWidth is the guard for the derived initial
|
||||
// offset (F15): at the default window width the divider must open at the
|
||||
// sidebar's own width — enough that the toolbar is never born clipped, and no
|
||||
// more, since every extra pixel is taken from the details pane. Split's own
|
||||
// clamp guarantees the lower bound, so the upper bound is what actually proves
|
||||
// the offset was derived rather than left at the 0.5 default.
|
||||
func TestJobsSplitOpensAtTheSidebarWidth(t *testing.T) {
|
||||
testApp := test.NewApp()
|
||||
defer testApp.Quit()
|
||||
w := testApp.NewWindow("test")
|
||||
defer w.Close()
|
||||
|
||||
store := newTestStore(t)
|
||||
svc := app.NewService(store, nil)
|
||||
defer svc.Stop()
|
||||
|
||||
content, _ := newJobsView(w, svc)
|
||||
w.SetContent(content)
|
||||
|
||||
split := jobsSplit(t, content)
|
||||
split.Resize(fyne.NewSize(defaultWindowWidth, defaultWindowHeight))
|
||||
|
||||
want := split.Leading.MinSize().Width
|
||||
got := split.Leading.Size().Width
|
||||
// One pixel of slack for the float32 round trip through the offset ratio.
|
||||
if got < want || got > want+1 {
|
||||
t.Errorf("leading pane opens at %v, want its content minimum %v", got, want)
|
||||
}
|
||||
if trailing := split.Trailing.Size().Width; trailing < split.Trailing.MinSize().Width {
|
||||
t.Errorf("trailing pane opens at %v, below its minimum %v", trailing, split.Trailing.MinSize().Width)
|
||||
}
|
||||
}
|
||||
|
||||
// TestToolbarButtonRedrawsRowAndDetails is the regression guard for F12: the
|
||||
// toolbar handlers no longer re-read the service or refresh the list
|
||||
// themselves, so refreshView alone has to re-snapshot the jobs and repopulate
|
||||
|
||||
@@ -59,6 +59,25 @@ func (l minWidthLayout) Layout(objects []fyne.CanvasObject, size fyne.Size) {
|
||||
// than hard-coded so it follows a theme that changes SizeNameInnerPadding.
|
||||
func rowOverlap() float32 { return -theme.InnerPadding() }
|
||||
|
||||
// initialSplitOffset returns the container.Split offset that opens a horizontal
|
||||
// split with its leading pane at the given natural width. SetOffset takes a
|
||||
// ratio, but a pane's natural width is absolute, so the ratio is derived from
|
||||
// the window width the app opens at rather than written as a literal: 0.44 fits
|
||||
// 1024 px but would hand a 448 px sidebar 700 px on a 1600 px-wide window.
|
||||
//
|
||||
// The divider sits between the panes and is excluded from the ratio, matching
|
||||
// container.Split's own arithmetic (its divider is two theme paddings thick).
|
||||
// Split clamps the offset to both panes' minimums when it lays out, so a result
|
||||
// that is slightly off — the window is a little wider than its content area —
|
||||
// costs at most a few pixels and can never clip either pane.
|
||||
func initialSplitOffset(leadingWidth float32) float64 {
|
||||
available := float64(defaultWindowWidth - 2*theme.Padding())
|
||||
if available <= 0 {
|
||||
return 0
|
||||
}
|
||||
return float64(leadingWidth) / available
|
||||
}
|
||||
|
||||
// fixedHeightLayout forces its contents to a fixed height while leaving the
|
||||
// width to the parent container. It is used to reserve a stable amount of space
|
||||
// for the activity panel so a neighbouring widget can absorb the rest.
|
||||
@@ -94,6 +113,13 @@ func (l fixedHeightLayout) Layout(objects []fyne.CanvasObject, size fyne.Size) {
|
||||
// stops it from growing with the window (as an even two-column grid would), so
|
||||
// the extra space a wider window provides goes entirely to the value column. It
|
||||
// expects exactly two children: caption first, value second.
|
||||
//
|
||||
// The value column has no minimum of its own: it gets whatever the container's
|
||||
// width leaves, down to zero. What keeps it readable in the details pane is the
|
||||
// 460 px minimum on commandOutputScroll (jobs_view_details.go), which is that
|
||||
// pane's widest minimum and therefore its floor — a constant that exists for an
|
||||
// unrelated reason. A new caller that gives this layout less width gets a value
|
||||
// column that silently renders narrow or empty rather than one that clips.
|
||||
type captionValueLayout struct {
|
||||
captionWidth float32
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user