Compare commits
5 Commits
e05adc1703
...
4d014e2f45
| Author | SHA1 | Date | |
|---|---|---|---|
| 4d014e2f45 | |||
| 706aa8e6ba | |||
| bfb982cc27 | |||
| 29ce94c3e8 | |||
| e85cbc4eb1 |
@@ -0,0 +1,26 @@
|
|||||||
|
---
|
||||||
|
description: Review the project as a whole against the agenda in docs/REVIEW.md
|
||||||
|
---
|
||||||
|
|
||||||
|
Perform a whole-project review of GoSentry.
|
||||||
|
|
||||||
|
Read [docs/REVIEW.md](../../docs/REVIEW.md) first — it is the agenda, and its
|
||||||
|
nine sections are the areas to cover. Read [docs/STANDARDS.md](../../docs/STANDARDS.md)
|
||||||
|
and [docs/ARCHITECTURE.md](../../docs/ARCHITECTURE.md) for the rules and
|
||||||
|
contracts the code is checked against.
|
||||||
|
|
||||||
|
$ARGUMENTS narrows the review when given — a package path, a file, or the name
|
||||||
|
of an agenda section. With no arguments, sweep the whole `src/` tree.
|
||||||
|
|
||||||
|
Rules for the report:
|
||||||
|
|
||||||
|
- Anything listed under "Intentional behavior" in STANDARDS.md is not a finding.
|
||||||
|
If you believe such an entry is now wrong, say so explicitly as a challenge to
|
||||||
|
the decision rather than reporting it as a bug.
|
||||||
|
- Verify before reporting. Read the surrounding code and, where cheap, confirm
|
||||||
|
the behavior with a test rather than reasoning about it alone.
|
||||||
|
- Group findings by agenda section, most severe first, each with the file and
|
||||||
|
line and what would actually go wrong.
|
||||||
|
- Report honestly that a section is clean rather than inventing something for it.
|
||||||
|
- Do not fix anything during the review. Report first; apply fixes only when
|
||||||
|
asked, following "What happens to the findings" in REVIEW.md.
|
||||||
@@ -20,6 +20,10 @@ logs/
|
|||||||
go.work
|
go.work
|
||||||
go.work.sum
|
go.work.sum
|
||||||
|
|
||||||
|
# Claude Code per-developer settings. The shared command in .claude/commands is
|
||||||
|
# tracked; locally granted tool permissions are not.
|
||||||
|
.claude/settings.local.json
|
||||||
|
|
||||||
# GoodSync metadata. This is intentionally kept because the directory is local
|
# GoodSync metadata. This is intentionally kept because the directory is local
|
||||||
# to the user's file synchronization setup.
|
# to the user's file synchronization setup.
|
||||||
_gsdata_/
|
_gsdata_/
|
||||||
|
|||||||
@@ -0,0 +1,59 @@
|
|||||||
|
# GoSentry — instructions for Claude Code
|
||||||
|
|
||||||
|
Cross-platform desktop scheduler (Go + Fyne GUI). Single process: GUI,
|
||||||
|
application service, scheduler, storage, and command runner in one binary.
|
||||||
|
|
||||||
|
## Read before changing code
|
||||||
|
|
||||||
|
- [docs/STANDARDS.md](docs/STANDARDS.md) — **required.** Code-quality rules and
|
||||||
|
the list of intentional behavior. Do not "fix" anything listed there as
|
||||||
|
intentional; if a change contradicts it, update the document in the same commit.
|
||||||
|
- [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md) — package contracts and event flow.
|
||||||
|
- [docs/TESTS.md](docs/TESTS.md) — test layout and conventions.
|
||||||
|
- [docs/ROADMAP.md](docs/ROADMAP.md) — deliberately out of scope.
|
||||||
|
|
||||||
|
## Reviewing the project
|
||||||
|
|
||||||
|
When the user asks for a review of the project (rather than of a specific
|
||||||
|
diff), follow [docs/REVIEW.md](docs/REVIEW.md) — it is the agenda, and the
|
||||||
|
`/review-project` command runs the same thing. Do not improvise a checklist.
|
||||||
|
|
||||||
|
## Key rules (full list in STANDARDS.md)
|
||||||
|
|
||||||
|
- `src/app.Service` is the sole owner of job and runtime state; the UI reads it
|
||||||
|
through typed events, never through shared mutable state.
|
||||||
|
- User-facing errors go to `dialog.ShowError` or a History event — never a silent
|
||||||
|
`return`.
|
||||||
|
- Pure helpers get a unit test in the same package; fixes of severity ≥ medium get
|
||||||
|
a regression test.
|
||||||
|
- UI view constructors accept an injected `*app.Service`; `app.Open()` is called
|
||||||
|
only from `run.go`.
|
||||||
|
- Off-main-thread widget updates must go through `fyne.Do` (Fyne v2.7.4).
|
||||||
|
|
||||||
|
## Build and test
|
||||||
|
|
||||||
|
CGO is required — the Fyne GUI links native libraries. On Windows the toolchain
|
||||||
|
is MSYS2 UCRT64; the default shell environment here has CGO off, so set it
|
||||||
|
explicitly:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
$env:Path = 'C:\msys64\ucrt64\bin;' + $env:Path; $env:CGO_ENABLED = '1'
|
||||||
|
```
|
||||||
|
|
||||||
|
Then:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
scripts\test.bat
|
||||||
|
```
|
||||||
|
|
||||||
|
which runs `go vet ./...` and `go test -race ./...`. Release binaries come from
|
||||||
|
`scripts\build-windows.bat` / `scripts/build-linux.sh` — see
|
||||||
|
[docs/DEVELOPMENT.md](docs/DEVELOPMENT.md).
|
||||||
|
|
||||||
|
## Repository conventions
|
||||||
|
|
||||||
|
- Commit directly to `main`; do not create feature branches.
|
||||||
|
- Notable changes get a [docs/CHANGELOG.md](docs/CHANGELOG.md) entry under the
|
||||||
|
current version.
|
||||||
|
- The window/taskbar icon comes from the `gosentry.ico` PE resource — regenerate
|
||||||
|
it from the PNGs whenever an icon changes, not just the embedded asset.
|
||||||
@@ -49,6 +49,7 @@ GoSentry is built and tested on **Windows** and **Linux**:
|
|||||||
- [Roadmap](docs/ROADMAP.md) — planned work larger than a single bug fix
|
- [Roadmap](docs/ROADMAP.md) — planned work larger than a single bug fix
|
||||||
- [Architecture](docs/ARCHITECTURE.md) — component interaction model
|
- [Architecture](docs/ARCHITECTURE.md) — component interaction model
|
||||||
- [Standards](docs/STANDARDS.md) — quality rules and intentional behavior
|
- [Standards](docs/STANDARDS.md) — quality rules and intentional behavior
|
||||||
|
- [Review](docs/REVIEW.md) — what a whole-project review looks at
|
||||||
- [Development](docs/DEVELOPMENT.md) — build instructions, project layout, dependencies
|
- [Development](docs/DEVELOPMENT.md) — build instructions, project layout, dependencies
|
||||||
- [Tests](docs/TESTS.md) — test suite layout and how to run it
|
- [Tests](docs/TESTS.md) — test suite layout and how to run it
|
||||||
- [Performance](docs/PERFORMANCE.md) — measured performance findings
|
- [Performance](docs/PERFORMANCE.md) — measured performance findings
|
||||||
|
|||||||
+16
-5
@@ -131,10 +131,21 @@ in flight increments `JobRuntime.PendingRuns`. When the current run finishes,
|
|||||||
|
|
||||||
### Per-job command timeout
|
### Per-job command timeout
|
||||||
|
|
||||||
`domain.Job` carries a `TimeoutSeconds` field (`json:"timeout_seconds,omitempty"`),
|
`domain.Job` carries a `TimeoutSeconds *int` field
|
||||||
following the same inherit pattern as the overlap policy. `0` means inherit the
|
(`json:"timeout_seconds,omitempty"`), following the same inherit pattern as the
|
||||||
global `Config.DefaultTimeoutSeconds` (default **0**, i.e. no timeout); a
|
overlap policy. It is a **pointer** because the setting has three states that
|
||||||
positive value overrides it for that job alone. `app.Service.effectiveTimeout`
|
must stay distinguishable on disk:
|
||||||
|
|
||||||
|
| `Job.TimeoutSeconds` | jobs.json | Meaning |
|
||||||
|
| --- | --- | --- |
|
||||||
|
| `nil` | field absent | inherit `Config.DefaultTimeoutSeconds` |
|
||||||
|
| `0` | `"timeout_seconds": 0` | no timeout, does **not** inherit |
|
||||||
|
| `> 0` | `"timeout_seconds": 45` | per-job limit in seconds |
|
||||||
|
|
||||||
|
The global `Config.DefaultTimeoutSeconds` (default **0**, i.e. no timeout) is
|
||||||
|
written unconditionally — no `omitempty` — for the same reason: `0` there is a
|
||||||
|
deliberate choice, not a missing value, and `storage.loadOrCreateConfig` must not
|
||||||
|
normalize it away. `app.Service.effectiveTimeout`
|
||||||
resolves the effective duration under `mu` and `startRunLocked` snapshots it into
|
resolves the effective duration under `mu` and `startRunLocked` snapshots it into
|
||||||
`runEnv.timeout`. `runner.RunJob(ctx, job, trigger, logsDir, timeout)` takes the
|
`runEnv.timeout`. `runner.RunJob(ctx, job, trigger, logsDir, timeout)` takes the
|
||||||
resolved duration as an argument, so the runner stays ignorant of the global
|
resolved duration as an argument, so the runner stays ignorant of the global
|
||||||
@@ -192,4 +203,4 @@ size guideline:
|
|||||||
|------|----------|
|
|------|----------|
|
||||||
| `jobs_view.go` | `newJobsView` — list, toolbar, button wiring, and layout |
|
| `jobs_view.go` | `newJobsView` — list, toolbar, button wiring, and layout |
|
||||||
| `jobs_view_details.go` | `detailsPanel` struct — widget creation, `update`, `clear`, `container` |
|
| `jobs_view_details.go` | `detailsPanel` struct — widget creation, `update`, `clear`, `container` |
|
||||||
| `jobs_view_helpers.go` | Pure helpers — `filteredJobIndexes`, `folderOptions`, `filterValue`, `indexOfID`, `lastJobLogs` |
|
| `jobs_view_helpers.go` | Pure helpers — `filteredJobIndexes`, `folderOptions`, `filterValue`, `indexOfID`, `lastJobLogs`, `nextJobListView`, `viewToggleText` |
|
||||||
|
|||||||
@@ -2,6 +2,37 @@
|
|||||||
|
|
||||||
All notable GoSentry changes are recorded in this file.
|
All notable GoSentry changes are recorded in this file.
|
||||||
|
|
||||||
|
## Unreleased
|
||||||
|
|
||||||
|
**Compact job list view.**
|
||||||
|
|
||||||
|
- The Jobs sidebar can now render each job as a single line — name on the left,
|
||||||
|
status on the right — instead of the three-line block. A toggle button beside
|
||||||
|
the Folder filter switches between **Compact** and **Detailed**; it is
|
||||||
|
labelled with the action it performs, like the "Disable auto" button. Compact
|
||||||
|
fits many more jobs on screen without scrolling; selection, the details panel,
|
||||||
|
the folder filter, and live status updates all work unchanged in both modes.
|
||||||
|
- The choice is persisted as a new `Config.JobListView` field
|
||||||
|
(`"detailed"` / `"compact"`, written to `gosentry.json` as `job_list_view`),
|
||||||
|
so it survives a restart. Empty/legacy configs and any unrecognised value
|
||||||
|
normalize to detailed, so existing installs keep the current look.
|
||||||
|
|
||||||
|
**Timeouts: 0 now means "no timeout" at both levels.**
|
||||||
|
|
||||||
|
- The global **Default timeout** in Settings now defaults to `0`, meaning jobs
|
||||||
|
run to completion with no deadline instead of being killed after 30s.
|
||||||
|
- A per-job timeout of `0` now also means "no timeout" and no longer inherits
|
||||||
|
the global default. Leaving the job's timeout **empty** is what inherits.
|
||||||
|
`Job.TimeoutSeconds` became `*int` so the three states — unset, explicit 0,
|
||||||
|
and a positive limit — stay distinguishable in `jobs.json`.
|
||||||
|
- Fixed: a global default of `0` did not survive a restart. `gosentry.json` was
|
||||||
|
loaded with `0` treated as a missing value and silently reset to 30s, so the
|
||||||
|
setting only held for the current session. `default_timeout_seconds` is now
|
||||||
|
written unconditionally and read back as-is.
|
||||||
|
|
||||||
|
Existing jobs and configs are unaffected: a job with no `timeout_seconds` still
|
||||||
|
inherits, and a saved global default of 30 stays 30.
|
||||||
|
|
||||||
## 0.13.0 - 2026-07-26
|
## 0.13.0 - 2026-07-26
|
||||||
|
|
||||||
**Branded GoSentry color theme; Cancel/Defaults buttons in Settings.**
|
**Branded GoSentry color theme; Cancel/Defaults buttons in Settings.**
|
||||||
|
|||||||
@@ -1,151 +0,0 @@
|
|||||||
# Implementation plan — compact / detailed job list view
|
|
||||||
|
|
||||||
## Context
|
|
||||||
|
|
||||||
The Jobs tab sidebar renders every job as a three-line block (bold name, a
|
|
||||||
metadata line with folder + schedule + command, and a status line) — see the
|
|
||||||
`widget.NewList` call in [jobs_view.go:108](../src/ui/jobs_view.go). That is
|
|
||||||
informative but tall: with many jobs the sidebar needs constant scrolling.
|
|
||||||
|
|
||||||
Add a second, opt-in rendering of the same list: **Compact** shows one line per
|
|
||||||
job (name left, status right); **Detailed** is the current three-line block and
|
|
||||||
stays the default. The choice is switchable from the Jobs tab itself and is
|
|
||||||
persisted in `gosentry.json`, so it survives a restart like `Config.Theme` does.
|
|
||||||
|
|
||||||
## Design decisions
|
|
||||||
|
|
||||||
- **Control lives in the Jobs sidebar header** — a toggle button on the same row
|
|
||||||
as the existing "Folder" filter, to its right. Switching is one click, next to
|
|
||||||
what it affects. Fyne 2.7.4 has no dedicated toggle widget, so the button
|
|
||||||
flips its own text and icon on tap, exactly like `stopAllButton`
|
|
||||||
("Disable auto" / "Enable auto") already does in `src/ui/jobs_view.go`. Like
|
|
||||||
that button it is labelled with the *action*, not the current state:
|
|
||||||
`Compact` + `theme.ListIcon()` while detailed, `Detailed` +
|
|
||||||
`theme.ViewFullScreenIcon()` while compact. The toolbar row (New job / Edit /
|
|
||||||
Run now / Pause / Delete) is left untouched.
|
|
||||||
- **Compact row = name + status** on a single line. Same height as one label,
|
|
||||||
so per-job health stays visible at a glance.
|
|
||||||
- **One list, one row template.** `widget.List` caches the row template's
|
|
||||||
`MinSize` as `itemMin`; `list.Refresh()` re-creates the template and
|
|
||||||
recomputes it (verified in Fyne 2.7.4 `widget/list.go`,
|
|
||||||
`listRenderer.Refresh`), so the rows genuinely shrink/grow on a mode switch
|
|
||||||
without rebuilding the widget. The template therefore always holds the same
|
|
||||||
four objects, and the mode is expressed by `Show()`/`Hide()`. Both
|
|
||||||
`compactVBoxLayout` (`src/ui/layout.go`) and Fyne's border layout skip hidden
|
|
||||||
children in `MinSize`, so a hidden line contributes no height.
|
|
||||||
|
|
||||||
## Changes
|
|
||||||
|
|
||||||
### 1. `src/domain/config.go` — persisted preference
|
|
||||||
|
|
||||||
- New string type `JobListView` with `JobListViewDetailed = "detailed"` and
|
|
||||||
`JobListViewCompact = "compact"`, documented like `Theme` as a UI-only choice.
|
|
||||||
- New field `JobListView JobListView \`json:"job_list_view,omitempty"\`` on
|
|
||||||
`Config`; comment that empty means detailed so pre-existing configs keep the
|
|
||||||
current look.
|
|
||||||
- `DefaultConfig()` returns `JobListViewDetailed`.
|
|
||||||
- Method `func (v JobListView) IsCompact() bool` — only the exact `"compact"`
|
|
||||||
value is compact — so every consumer normalizes unknown/legacy values the
|
|
||||||
same way.
|
|
||||||
|
|
||||||
### 2. `src/app/operations.go` — save the choice
|
|
||||||
|
|
||||||
New `func (s *Service) SetJobListView(view domain.JobListView) error`, modelled
|
|
||||||
on `SetGlobalPause` (`src/app/operations.go:168`) but deliberately lighter:
|
|
||||||
normalize a non-`compact` value to `JobListViewDetailed`, take `mu`, return
|
|
||||||
early if unchanged, set `s.store.Config.JobListView`, call
|
|
||||||
`s.store.SaveConfig()`, unlock. No `SaveJobs` (no job changed), no event emitted
|
|
||||||
(this is presentational only — the Jobs view refreshes its own list; an event
|
|
||||||
would trigger a pointless whole-window refresh). A comment records that
|
|
||||||
reasoning.
|
|
||||||
|
|
||||||
### 3. `src/ui/jobs_view.go` — rendering and the toggle
|
|
||||||
|
|
||||||
- Read the initial mode into a `compactList` local:
|
|
||||||
`svc.Store().Config.JobListView.IsCompact()`.
|
|
||||||
- **CreateItem**: build `name` (bold, `Wrapping = fyne.TextTruncate` so a long
|
|
||||||
name cannot push the status off the row), `inlineStatus` (compact-only,
|
|
||||||
right-hand side), `meta`, and `status`. Put the first two on one line via
|
|
||||||
`container.NewBorder(nil, nil, nil, inlineStatus, name)` and keep the outer
|
|
||||||
`compactVBoxLayout{spacing: jobRowSpacing}` container with
|
|
||||||
`[nameLine, meta, status]`. Apply the mode's visibility here too — this is
|
|
||||||
what makes `itemMin` correct for the mode.
|
|
||||||
- **UpdateItem**: reach `nameLine` through `row.Objects[0].(*fyne.Container)`
|
|
||||||
(`NewBorder` appends the border slots after the center object, so
|
|
||||||
`Objects[0]` is `name` and `Objects[1]` is `inlineStatus` — worth a comment,
|
|
||||||
matching the existing index-based access in this callback). Set all four texts
|
|
||||||
from the existing helpers (`app.DisplayFolder`, `app.DisplayInvocation`,
|
|
||||||
`app.StatusText`) and **re-apply visibility on every update**: a full
|
|
||||||
`Refresh` reuses the already-visible row objects, which were created under the
|
|
||||||
old mode, so visibility cannot be left to `CreateItem` alone.
|
|
||||||
- **View toggle button**: `viewButton := widget.NewButtonWithIcon(…)` built with
|
|
||||||
the text/icon for the current mode. Its `OnTapped` flips the mode
|
|
||||||
(`nextJobListView`), persists via `svc.SetJobListView` — on error
|
|
||||||
`dialog.ShowError` and roll the local mode back without touching the button,
|
|
||||||
per the error rule in `docs/STANDARDS.md` — then applies the new
|
|
||||||
`SetText`/`SetIcon` and calls `list.Refresh()`. This mirrors `stopAllButton`'s
|
|
||||||
flip-and-revert handler in the same file. The list selection and the details
|
|
||||||
panel are untouched by the switch.
|
|
||||||
- **Header layout**: keep the `"Folder"` caption and `folderSelect` exactly as
|
|
||||||
they are, and put the button beside the select on the same row —
|
|
||||||
`container.NewBorder(nil, nil, nil, viewButton, folderSelect)`. The border
|
|
||||||
layout gives the button its `MinSize` on the right and lets the select fill
|
|
||||||
the rest, so the header gains no height and the toolbar row is not touched.
|
|
||||||
|
|
||||||
### 4. `src/ui/jobs_view_helpers.go` — button state helpers
|
|
||||||
|
|
||||||
Two pure helpers so the on-disk strings never reach the user and the button's
|
|
||||||
wording is testable without a running GUI:
|
|
||||||
|
|
||||||
- `nextJobListView(current domain.JobListView) domain.JobListView` — flips the
|
|
||||||
mode, treating anything that is not `compact` as detailed (via `IsCompact`).
|
|
||||||
- `viewToggleText(current domain.JobListView) string` — the action label:
|
|
||||||
`"Compact"` while detailed, `"Detailed"` while compact.
|
|
||||||
|
|
||||||
The icon choice stays inline at the button, next to `SetText`/`SetIcon`, so the
|
|
||||||
helpers file keeps its "no widget imports" character.
|
|
||||||
|
|
||||||
### 5. Tests
|
|
||||||
|
|
||||||
- `src/domain/config_test.go` (new): `JobListView.IsCompact` for `"compact"`,
|
|
||||||
`"detailed"`, `""`, and a junk value.
|
|
||||||
- `src/ui/jobs_view_test.go`: `nextJobListView` flips both ways and maps an
|
|
||||||
empty/unknown value to compact (since such a value reads as detailed), and
|
|
||||||
`viewToggleText` returns the action label for each mode.
|
|
||||||
- `src/app/operations_test.go`: `TestSetJobListViewPersistsToConfigFile`
|
|
||||||
modelled on `TestSetGlobalPausePersistsToConfigFile`
|
|
||||||
(`src/app/operations_test.go:553`) — switch to compact, unmarshal
|
|
||||||
`svc.store.Paths.ConfigPath`, assert the field, switch back; plus a case
|
|
||||||
asserting an unknown value is stored as `detailed`.
|
|
||||||
- `src/storage/store_test.go`: one added assertion in the defaults test beside
|
|
||||||
the existing `Theme` check (~line 174) that a fresh config is `detailed`.
|
|
||||||
|
|
||||||
### 6. Docs and version
|
|
||||||
|
|
||||||
- `docs/CHANGELOG.md`: new `## 0.14.0 - 2026-07-26` section (a feature → minor
|
|
||||||
bump) covering the compact/detailed toggle and the new `job_list_view` field,
|
|
||||||
noting legacy configs stay detailed.
|
|
||||||
- `src/app/version.go`: `0.13.0` → `0.14.0`.
|
|
||||||
- `docs/ARCHITECTURE.md`: add the two new helpers to the `jobs_view_helpers.go`
|
|
||||||
row of the file-split table (~line 195).
|
|
||||||
|
|
||||||
## Verification
|
|
||||||
|
|
||||||
1. Build, vet, and test the whole module through the cgo-enabled shell (the
|
|
||||||
default Bash env has CGO off, so GUI packages will not compile there):
|
|
||||||
|
|
||||||
```bash
|
|
||||||
export PATH="/c/msys64/ucrt64/bin:$PATH"; export CGO_ENABLED=1; go build ./... && go vet ./... && go test -race ./...
|
|
||||||
```
|
|
||||||
|
|
||||||
2. Run the app (`go run ./cmd/gosentry`) with several jobs configured and check:
|
|
||||||
- Default launch is Detailed and looks exactly as before; the button reads
|
|
||||||
"Compact" and sits to the right of the folder filter without making the
|
|
||||||
header taller.
|
|
||||||
- Tapping it collapses every row to one line, name left / status right, many
|
|
||||||
more jobs fit without scrolling, and the button now reads "Detailed".
|
|
||||||
- Selection and the details panel keep working after switching, in both
|
|
||||||
directions, including with a folder filter active and with an empty list.
|
|
||||||
- A running job's status still updates live in compact rows.
|
|
||||||
- `gosentry.json` gains `"job_list_view": "compact"`; restarting reopens in
|
|
||||||
compact, and switching back removes/flips the field.
|
|
||||||
@@ -0,0 +1,86 @@
|
|||||||
|
# GoSentry — Review Agenda
|
||||||
|
|
||||||
|
What to look at when reviewing the project as a whole, as opposed to a single
|
||||||
|
diff. This is the agenda; the rules a review checks against live in
|
||||||
|
[STANDARDS.md](STANDARDS.md) and [ARCHITECTURE.md](ARCHITECTURE.md).
|
||||||
|
|
||||||
|
Scope note: a normal pull-request review checks the change. This agenda is for
|
||||||
|
a periodic sweep of the whole codebase, so a pass may legitimately end with
|
||||||
|
"nothing to report" on most items.
|
||||||
|
|
||||||
|
## 1. Architecture and project structure
|
||||||
|
|
||||||
|
Does the code still match the package map and the event flow in
|
||||||
|
[ARCHITECTURE.md](ARCHITECTURE.md)? Watch for the boundaries that matter here:
|
||||||
|
`app.Service` as the sole owner of job and runtime state, the UI reading it
|
||||||
|
through typed events, `domain` staying free of I/O, and platform-specific code
|
||||||
|
staying behind the `platform/*` interfaces.
|
||||||
|
|
||||||
|
## 2. Complexity against the size of the project
|
||||||
|
|
||||||
|
GoSentry is a single-process desktop app with two direct dependencies. Flag
|
||||||
|
abstraction that is not paying for itself: interfaces with one implementation
|
||||||
|
and no test seam, indirection added for a use case nobody has asked for, a new
|
||||||
|
dependency where thirty lines of standard library would do. Also check the
|
||||||
|
opposite direction — files that have grown past the size guideline in
|
||||||
|
ARCHITECTURE and should be split the way `jobs_view.go` was.
|
||||||
|
|
||||||
|
## 3. Code quality
|
||||||
|
|
||||||
|
The checkable rules are in [STANDARDS.md](STANDARDS.md) — error handling, unit
|
||||||
|
tests for pure helpers, regression tests for fixes, `fyne.Do` for updates off
|
||||||
|
the main thread. Beyond them: concurrency around `Service.mu`, goroutines whose
|
||||||
|
lifetime is not obvious, and error paths that report something less useful than
|
||||||
|
what they caught.
|
||||||
|
|
||||||
|
## 4. Documentation and comments
|
||||||
|
|
||||||
|
Does every documented behavior still exist, and does every non-obvious behavior
|
||||||
|
get documented? Check the doc set against the code: README (user-facing
|
||||||
|
behavior and config keys), ARCHITECTURE (packages and flows), STANDARDS
|
||||||
|
(rules and intentional behavior), DEVELOPMENT (build), TESTS, PERFORMANCE,
|
||||||
|
CHANGELOG (an entry per notable change). For comments, the bar is *why*, not
|
||||||
|
*what* — a comment restating the line below it is noise; an unexplained
|
||||||
|
workaround is a finding.
|
||||||
|
|
||||||
|
## 5. Readability and maintainability
|
||||||
|
|
||||||
|
Read a package as someone who has not seen it before. Can the next change be
|
||||||
|
made without reverse-engineering? Naming that matches the domain vocabulary,
|
||||||
|
functions that do one thing, and control flow that does not need a diagram.
|
||||||
|
|
||||||
|
## 6. Logical errors
|
||||||
|
|
||||||
|
Correctness independent of style: scheduling and timing edge cases (overlap
|
||||||
|
policy, sequential mode, pause interactions), off-by-one and boundary handling,
|
||||||
|
zero values that mean something (see the timeout rules in STANDARDS), state
|
||||||
|
that can be observed mid-update, and error paths that leave state inconsistent.
|
||||||
|
|
||||||
|
## 7. Legacy code and migrations
|
||||||
|
|
||||||
|
The app has no database, so migration means file compatibility: `gosentry.json`
|
||||||
|
and `jobs.json` written by an older version must keep working. Check that new
|
||||||
|
`Config` fields are backward compatible, that normalization happens in one
|
||||||
|
place, and that values which are meaningful zeros are not normalized away. Also
|
||||||
|
look for code kept alive only for a case that no longer exists.
|
||||||
|
|
||||||
|
## 8. Undocumented or under-documented contentious decisions
|
||||||
|
|
||||||
|
Any decision a future reader would question needs its reasoning recorded where
|
||||||
|
it lives: a comment at the code, an entry in the "Intentional behavior" section
|
||||||
|
of [STANDARDS.md](STANDARDS.md), or — when the work is deferred rather than
|
||||||
|
decided — a note in [ROADMAP.md](ROADMAP.md), which is where the frozen
|
||||||
|
window-size work keeps its rationale.
|
||||||
|
|
||||||
|
## 9. Other improvement proposals
|
||||||
|
|
||||||
|
Anything that does not fit above: build and release ergonomics, test coverage
|
||||||
|
gaps, dependency health, UX rough edges.
|
||||||
|
|
||||||
|
## What happens to the findings
|
||||||
|
|
||||||
|
- A defect → fix it, with a regression test when severity is medium or higher.
|
||||||
|
- Behavior that turns out to be deliberate → record it under "Intentional
|
||||||
|
behavior" in [STANDARDS.md](STANDARDS.md) so it is not re-reported.
|
||||||
|
- Work larger than a single fix → [ROADMAP.md](ROADMAP.md), with the reasoning.
|
||||||
|
- A new rule the review establishes → [STANDARDS.md](STANDARDS.md).
|
||||||
+24
-3
@@ -1,7 +1,8 @@
|
|||||||
# GoSentry — Standards
|
# GoSentry — Standards
|
||||||
|
|
||||||
Quality rules and intentional behavior for contributors. Package contracts live
|
Quality rules and intentional behavior for contributors. Package contracts live
|
||||||
in [ARCHITECTURE.md](ARCHITECTURE.md); test conventions in [TESTS.md](TESTS.md).
|
in [ARCHITECTURE.md](ARCHITECTURE.md); test conventions in [TESTS.md](TESTS.md);
|
||||||
|
what a whole-project review looks at, in [REVIEW.md](REVIEW.md).
|
||||||
|
|
||||||
## Code quality
|
## Code quality
|
||||||
|
|
||||||
@@ -12,14 +13,34 @@ in [ARCHITECTURE.md](ARCHITECTURE.md); test conventions in [TESTS.md](TESTS.md).
|
|||||||
- Documented intentional behavior → section below, not a backlog bug.
|
- Documented intentional behavior → section below, not a backlog bug.
|
||||||
- UI view constructors accept `*app.Service`; call `app.Open()` only from `run.go`.
|
- UI view constructors accept `*app.Service`; call `app.Open()` only from `run.go`.
|
||||||
|
|
||||||
|
## Config file compatibility
|
||||||
|
|
||||||
|
There is no migration step: `gosentry.json` and `jobs.json` are read as-is, are
|
||||||
|
meant to be hand-editable, and may have been written by an older version. A
|
||||||
|
change to their shape has to stay compatible on its own.
|
||||||
|
|
||||||
|
- A new `Config` field is tagged `omitempty`, and its zero value must mean the
|
||||||
|
behavior that existed before the field was added — a file written without it
|
||||||
|
keeps working unchanged. `DefaultConfig()` still sets the value explicitly.
|
||||||
|
- A zero that carries meaning is not a missing field and must not be backfilled
|
||||||
|
on load. See `DefaultTimeoutSeconds` in `storage.loadOrCreateConfig` and
|
||||||
|
`Job.TimeoutSeconds *int`, where unset and `0` are different answers.
|
||||||
|
- An unrecognised enum value reads as the default rather than an error, through
|
||||||
|
one helper that every consumer shares (`JobListView.IsCompact`, `ui.themeFor`),
|
||||||
|
and is normalized before being written back, so the file never gains a value
|
||||||
|
no reader understands.
|
||||||
|
- Each of the three gets a test: the default in `storage`, the normalization in
|
||||||
|
`domain`, and a round-trip through the real config file in `app`.
|
||||||
|
|
||||||
## Intentional behavior (not bugs)
|
## Intentional behavior (not bugs)
|
||||||
|
|
||||||
- `RunNow` is allowed during global pause and for disabled jobs.
|
- `RunNow` is allowed during global pause and for disabled jobs.
|
||||||
- Sequential mode runs jobs FIFO by order in `jobs.json`.
|
- Sequential mode runs jobs FIFO by order in `jobs.json`.
|
||||||
- Scheduler tick is 1s — sub-second `@every` intervals are not supported.
|
- Scheduler tick is 1s — sub-second `@every` intervals are not supported.
|
||||||
- Command timeout defaults to no timeout globally (`Config.DefaultTimeoutSeconds`
|
- Command timeout defaults to no timeout globally (`Config.DefaultTimeoutSeconds`
|
||||||
= 0) and is overridable per job (`Job.TimeoutSeconds`, 0 = inherit the global
|
= 0) and is overridable per job (`Job.TimeoutSeconds *int`: unset = inherit the
|
||||||
default).
|
global default, 0 = no timeout, positive = seconds). Neither zero may be
|
||||||
|
normalized away on load — 0 is a value, not a missing field.
|
||||||
- **History tab is session-only.** `JobRuntime.Logs` exists only in memory for the
|
- **History tab is session-only.** `JobRuntime.Logs` exists only in memory for the
|
||||||
current process. Log files on disk feed aggregate statistics via `SeedStats`
|
current process. Log files on disk feed aggregate statistics via `SeedStats`
|
||||||
only. See [ARCHITECTURE.md](ARCHITECTURE.md).
|
only. See [ARCHITECTURE.md](ARCHITECTURE.md).
|
||||||
|
|||||||
+9
-5
@@ -103,12 +103,16 @@ func DisplayOverlapPolicy(job domain.Job, globalPolicy domain.OverlapPolicy) str
|
|||||||
}
|
}
|
||||||
|
|
||||||
// DisplayTimeout formats a job's effective run timeout for the details panel.
|
// DisplayTimeout formats a job's effective run timeout for the details panel.
|
||||||
// When the job sets its own TimeoutSeconds it is shown as-is; when 0 (inherit),
|
// When the job sets its own TimeoutSeconds it is shown as-is, with an explicit 0
|
||||||
// the global default is shown with "(global default)" appended, mirroring
|
// rendered as "no timeout"; when unset (nil), the global default is shown with
|
||||||
// DisplayOverlapPolicy. A non-positive global default means no timeout at all.
|
// "(global default)" appended, mirroring DisplayOverlapPolicy. A non-positive
|
||||||
|
// global default means no timeout at all.
|
||||||
func DisplayTimeout(job domain.Job, globalDefault int) string {
|
func DisplayTimeout(job domain.Job, globalDefault int) string {
|
||||||
if job.TimeoutSeconds > 0 {
|
if job.TimeoutSeconds != nil {
|
||||||
return fmt.Sprintf("%d s", job.TimeoutSeconds)
|
if *job.TimeoutSeconds <= 0 {
|
||||||
|
return "no timeout"
|
||||||
|
}
|
||||||
|
return fmt.Sprintf("%d s", *job.TimeoutSeconds)
|
||||||
}
|
}
|
||||||
if globalDefault <= 0 {
|
if globalDefault <= 0 {
|
||||||
return "no timeout (global default)"
|
return "no timeout (global default)"
|
||||||
|
|||||||
@@ -165,11 +165,15 @@ func TestDisplayOverlapPolicy(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestDisplayTimeout(t *testing.T) {
|
func TestDisplayTimeout(t *testing.T) {
|
||||||
own := domain.Job{TimeoutSeconds: 45}
|
own := domain.Job{TimeoutSeconds: domain.TimeoutSecondsPtr(45)}
|
||||||
if got, want := DisplayTimeout(own, 30), "45 s"; got != want {
|
if got, want := DisplayTimeout(own, 30), "45 s"; got != want {
|
||||||
t.Errorf("per-job timeout = %q, want %q", got, want)
|
t.Errorf("per-job timeout = %q, want %q", got, want)
|
||||||
}
|
}
|
||||||
inherit := domain.Job{TimeoutSeconds: 0}
|
none := domain.Job{TimeoutSeconds: domain.TimeoutSecondsPtr(0)}
|
||||||
|
if got, want := DisplayTimeout(none, 30), "no timeout"; got != want {
|
||||||
|
t.Errorf("explicit per-job zero timeout = %q, want %q", got, want)
|
||||||
|
}
|
||||||
|
inherit := domain.Job{TimeoutSeconds: nil}
|
||||||
if got, want := DisplayTimeout(inherit, 30), "30 s (global default)"; got != want {
|
if got, want := DisplayTimeout(inherit, 30), "30 s (global default)"; got != want {
|
||||||
t.Errorf("inherited timeout = %q, want %q", got, want)
|
t.Errorf("inherited timeout = %q, want %q", got, want)
|
||||||
}
|
}
|
||||||
|
|||||||
+23
-2
@@ -193,6 +193,27 @@ func (s *Service) SetGlobalPause(paused bool) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SetJobListView persists the Jobs list density preference. Unlike
|
||||||
|
// SetGlobalPause this touches nothing but the config: no job changed, so there
|
||||||
|
// is no SaveJobs, and no event is emitted — the choice is presentational and the
|
||||||
|
// Jobs view refreshes its own list, whereas an event would trigger a pointless
|
||||||
|
// whole-window refresh. Anything that is not "compact" is stored as detailed so
|
||||||
|
// the file never gains an unrecognised value.
|
||||||
|
func (s *Service) SetJobListView(view domain.JobListView) error {
|
||||||
|
if !view.IsCompact() {
|
||||||
|
view = domain.JobListViewDetailed
|
||||||
|
}
|
||||||
|
s.mu.Lock()
|
||||||
|
if s.store.Config.JobListView == view {
|
||||||
|
s.mu.Unlock()
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
s.store.Config.JobListView = view
|
||||||
|
err := s.store.SaveConfig()
|
||||||
|
s.mu.Unlock()
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// ShouldNotifyOnFailure reports whether the user has enabled desktop
|
// ShouldNotifyOnFailure reports whether the user has enabled desktop
|
||||||
// notifications for failed job runs. It reads the config under mu so it is
|
// notifications for failed job runs. It reads the config under mu so it is
|
||||||
// safe to call from any goroutine.
|
// safe to call from any goroutine.
|
||||||
@@ -367,8 +388,8 @@ func validateJob(job domain.Job) error {
|
|||||||
if policy != "" && policy != string(domain.OverlapPolicySkip) && policy != string(domain.OverlapPolicyQueue) {
|
if policy != "" && policy != string(domain.OverlapPolicySkip) && policy != string(domain.OverlapPolicyQueue) {
|
||||||
return errors.New("overlap policy must be 'skip', 'queue', or empty")
|
return errors.New("overlap policy must be 'skip', 'queue', or empty")
|
||||||
}
|
}
|
||||||
if job.TimeoutSeconds < 0 {
|
if job.TimeoutSeconds != nil && *job.TimeoutSeconds < 0 {
|
||||||
return errors.New("timeout must be zero (inherit) or a positive number of seconds")
|
return errors.New("timeout must be zero (no timeout) or a positive number of seconds, or unset to inherit the global default")
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -103,9 +103,13 @@ func TestCreateJobValidates(t *testing.T) {
|
|||||||
if _, err := svc.CreateJob(domain.Job{Name: "A", Schedule: "@every 1m", Command: "echo", OverlapPolicy: "invalid"}); err == nil {
|
if _, err := svc.CreateJob(domain.Job{Name: "A", Schedule: "@every 1m", Command: "echo", OverlapPolicy: "invalid"}); err == nil {
|
||||||
t.Error("expected error for invalid overlap policy")
|
t.Error("expected error for invalid overlap policy")
|
||||||
}
|
}
|
||||||
if _, err := svc.CreateJob(domain.Job{Name: "A", Schedule: "@every 1m", Command: "echo", TimeoutSeconds: -1}); err == nil {
|
if _, err := svc.CreateJob(domain.Job{Name: "A", Schedule: "@every 1m", Command: "echo", TimeoutSeconds: domain.TimeoutSecondsPtr(-1)}); err == nil {
|
||||||
t.Error("expected error for negative per-job timeout")
|
t.Error("expected error for negative per-job timeout")
|
||||||
}
|
}
|
||||||
|
// An explicit 0 is a valid choice ("no timeout"), not a rejected one.
|
||||||
|
if _, err := svc.CreateJob(domain.Job{Name: "Zero", Schedule: "@every 1m", Command: "echo", TimeoutSeconds: domain.TimeoutSecondsPtr(0)}); err != nil {
|
||||||
|
t.Errorf("explicit zero per-job timeout should be accepted: %v", err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestUpdateJobKeepsRuntimeAndReflectsDisable(t *testing.T) {
|
func TestUpdateJobKeepsRuntimeAndReflectsDisable(t *testing.T) {
|
||||||
@@ -586,6 +590,63 @@ func TestSetGlobalPausePersistsToConfigFile(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSetJobListViewPersistsToConfigFile(t *testing.T) {
|
||||||
|
svc := newTempService(t, nil)
|
||||||
|
|
||||||
|
readConfig := func(stage string) domain.Config {
|
||||||
|
t.Helper()
|
||||||
|
data, err := os.ReadFile(svc.store.Paths.ConfigPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("reading config file %s: %v", stage, err)
|
||||||
|
}
|
||||||
|
var cfg domain.Config
|
||||||
|
if err := json.Unmarshal(data, &cfg); err != nil {
|
||||||
|
t.Fatalf("unmarshalling config %s: %v", stage, err)
|
||||||
|
}
|
||||||
|
return cfg
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := svc.SetJobListView(domain.JobListViewCompact); err != nil {
|
||||||
|
t.Fatalf("SetJobListView(compact): %v", err)
|
||||||
|
}
|
||||||
|
if got := readConfig("after compact").JobListView; got != domain.JobListViewCompact {
|
||||||
|
t.Errorf("persisted JobListView = %q, want %q", got, domain.JobListViewCompact)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := svc.SetJobListView(domain.JobListViewDetailed); err != nil {
|
||||||
|
t.Fatalf("SetJobListView(detailed): %v", err)
|
||||||
|
}
|
||||||
|
if got := readConfig("after detailed").JobListView; got != domain.JobListViewDetailed {
|
||||||
|
t.Errorf("persisted JobListView = %q, want %q", got, domain.JobListViewDetailed)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSetJobListViewNormalizesUnknownValue guards the config file against
|
||||||
|
// gaining a value no reader understands: anything but "compact" is stored as
|
||||||
|
// "detailed".
|
||||||
|
func TestSetJobListViewNormalizesUnknownValue(t *testing.T) {
|
||||||
|
svc := newTempService(t, nil)
|
||||||
|
|
||||||
|
if err := svc.SetJobListView(domain.JobListViewCompact); err != nil {
|
||||||
|
t.Fatalf("SetJobListView(compact): %v", err)
|
||||||
|
}
|
||||||
|
if err := svc.SetJobListView("tiny"); err != nil {
|
||||||
|
t.Fatalf("SetJobListView(tiny): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
data, err := os.ReadFile(svc.store.Paths.ConfigPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("reading config file: %v", err)
|
||||||
|
}
|
||||||
|
var cfg domain.Config
|
||||||
|
if err := json.Unmarshal(data, &cfg); err != nil {
|
||||||
|
t.Fatalf("unmarshalling config: %v", err)
|
||||||
|
}
|
||||||
|
if cfg.JobListView != domain.JobListViewDetailed {
|
||||||
|
t.Errorf("persisted JobListView = %q, want %q", cfg.JobListView, domain.JobListViewDetailed)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestServiceRebuiltFromPausedStoreStartsPaused(t *testing.T) {
|
func TestServiceRebuiltFromPausedStoreStartsPaused(t *testing.T) {
|
||||||
jobs := []domain.Job{{ID: 1, Name: "A", Schedule: "@every 1m", Command: "echo", Enabled: true}}
|
jobs := []domain.Job{{ID: 1, Name: "A", Schedule: "@every 1m", Command: "echo", Enabled: true}}
|
||||||
svc := newTempService(t, jobs)
|
svc := newTempService(t, jobs)
|
||||||
|
|||||||
+7
-6
@@ -211,15 +211,16 @@ func (s *Service) effectiveOverlapPolicy(job *domain.Job) domain.OverlapPolicy {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// effectiveTimeout resolves the run timeout that actually governs a job: the
|
// effectiveTimeout resolves the run timeout that actually governs a job: the
|
||||||
// job's own TimeoutSeconds when positive, otherwise the global
|
// job's own TimeoutSeconds whenever it is set — including an explicit 0, which
|
||||||
// Config.DefaultTimeoutSeconds. A non-positive Job.TimeoutSeconds means "inherit
|
// means "no timeout" and deliberately does not inherit — otherwise the global
|
||||||
// the global default", which is why normalizeJob leaves 0 rather than
|
// Config.DefaultTimeoutSeconds. A nil Job.TimeoutSeconds means "inherit the
|
||||||
|
// global default", which is why normalizeJob leaves it nil rather than
|
||||||
// backfilling the configured value. A resolved duration of 0 means no timeout;
|
// backfilling the configured value. A resolved duration of 0 means no timeout;
|
||||||
// runner.RunJob treats it as "run without a deadline". The caller must hold mu.
|
// runner.RunJob treats it as "run without a deadline". The caller must hold mu.
|
||||||
func (s *Service) effectiveTimeout(job *domain.Job) time.Duration {
|
func (s *Service) effectiveTimeout(job *domain.Job) time.Duration {
|
||||||
secs := job.TimeoutSeconds
|
secs := s.store.Config.DefaultTimeoutSeconds
|
||||||
if secs <= 0 {
|
if job.TimeoutSeconds != nil {
|
||||||
secs = s.store.Config.DefaultTimeoutSeconds
|
secs = *job.TimeoutSeconds
|
||||||
}
|
}
|
||||||
return time.Duration(secs) * time.Second
|
return time.Duration(secs) * time.Second
|
||||||
}
|
}
|
||||||
|
|||||||
+12
-5
@@ -617,23 +617,30 @@ func TestRunDueQueueDrainSkippedWhenPaused(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestEffectiveTimeout verifies the inherit-or-override resolution: a zero
|
// TestEffectiveTimeout verifies the three-state resolution: an unset (nil)
|
||||||
// Job.TimeoutSeconds falls back to the global default, while a positive value
|
// Job.TimeoutSeconds falls back to the global default, a positive value
|
||||||
// overrides it.
|
// overrides it, and an explicit 0 means "no timeout" without inheriting.
|
||||||
func TestEffectiveTimeout(t *testing.T) {
|
func TestEffectiveTimeout(t *testing.T) {
|
||||||
svc := newTempService(t, nil)
|
svc := newTempService(t, nil)
|
||||||
svc.store.Config.DefaultTimeoutSeconds = 30
|
svc.store.Config.DefaultTimeoutSeconds = 30
|
||||||
|
|
||||||
inherit := &domain.Job{TimeoutSeconds: 0}
|
inherit := &domain.Job{TimeoutSeconds: nil}
|
||||||
if got, want := svc.effectiveTimeout(inherit), 30*time.Second; got != want {
|
if got, want := svc.effectiveTimeout(inherit), 30*time.Second; got != want {
|
||||||
t.Errorf("inherited timeout = %s, want %s", got, want)
|
t.Errorf("inherited timeout = %s, want %s", got, want)
|
||||||
}
|
}
|
||||||
|
|
||||||
own := &domain.Job{TimeoutSeconds: 5}
|
own := &domain.Job{TimeoutSeconds: domain.TimeoutSecondsPtr(5)}
|
||||||
if got, want := svc.effectiveTimeout(own), 5*time.Second; got != want {
|
if got, want := svc.effectiveTimeout(own), 5*time.Second; got != want {
|
||||||
t.Errorf("per-job timeout = %s, want %s", got, want)
|
t.Errorf("per-job timeout = %s, want %s", got, want)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// An explicit per-job 0 must beat a positive global default rather than be
|
||||||
|
// mistaken for "unset".
|
||||||
|
none := &domain.Job{TimeoutSeconds: domain.TimeoutSecondsPtr(0)}
|
||||||
|
if got, want := svc.effectiveTimeout(none), time.Duration(0); got != want {
|
||||||
|
t.Errorf("explicit per-job zero timeout = %s, want %s (no timeout)", got, want)
|
||||||
|
}
|
||||||
|
|
||||||
svc.store.Config.DefaultTimeoutSeconds = 0
|
svc.store.Config.DefaultTimeoutSeconds = 0
|
||||||
if got, want := svc.effectiveTimeout(inherit), time.Duration(0); got != want {
|
if got, want := svc.effectiveTimeout(inherit), time.Duration(0); got != want {
|
||||||
t.Errorf("inherited timeout with no global default = %s, want %s (no timeout)", got, want)
|
t.Errorf("inherited timeout with no global default = %s, want %s (no timeout)", got, want)
|
||||||
|
|||||||
+30
-4
@@ -28,6 +28,26 @@ const (
|
|||||||
ThemeGoSentry Theme = "gosentry"
|
ThemeGoSentry Theme = "gosentry"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// JobListView selects how densely the Jobs tab renders its sidebar list. Like
|
||||||
|
// Theme it is a UI-only choice with no effect on scheduling; it lives in Config
|
||||||
|
// so the user's preference survives a restart.
|
||||||
|
type JobListView string
|
||||||
|
|
||||||
|
const (
|
||||||
|
// JobListViewDetailed is the three-line row: name, metadata, status.
|
||||||
|
JobListViewDetailed JobListView = "detailed"
|
||||||
|
// JobListViewCompact is the one-line row: name on the left, status on the
|
||||||
|
// right, so many more jobs fit without scrolling.
|
||||||
|
JobListViewCompact JobListView = "compact"
|
||||||
|
)
|
||||||
|
|
||||||
|
// IsCompact reports whether the compact rendering is selected. Only the exact
|
||||||
|
// "compact" value counts, so empty, legacy, and unrecognised values all read as
|
||||||
|
// detailed — every consumer normalizes them the same way.
|
||||||
|
func (v JobListView) IsCompact() bool {
|
||||||
|
return v == JobListViewCompact
|
||||||
|
}
|
||||||
|
|
||||||
// OverlapPolicy decides what happens when a job's next run fires while the
|
// OverlapPolicy decides what happens when a job's next run fires while the
|
||||||
// previous run is still active.
|
// previous run is still active.
|
||||||
type OverlapPolicy string
|
type OverlapPolicy string
|
||||||
@@ -53,14 +73,19 @@ type Config struct {
|
|||||||
NotifyOnFailure bool `json:"notify_on_failure,omitempty"`
|
NotifyOnFailure bool `json:"notify_on_failure,omitempty"`
|
||||||
ExecutionMode ExecutionMode `json:"execution_mode,omitempty"`
|
ExecutionMode ExecutionMode `json:"execution_mode,omitempty"`
|
||||||
OverlapPolicy OverlapPolicy `json:"overlap_policy,omitempty"`
|
OverlapPolicy OverlapPolicy `json:"overlap_policy,omitempty"`
|
||||||
// DefaultTimeoutSeconds is the run timeout applied to jobs that do not set
|
// DefaultTimeoutSeconds is the run timeout applied to jobs that leave their
|
||||||
// their own Job.TimeoutSeconds. 0 (the default) means no timeout: such jobs
|
// own Job.TimeoutSeconds unset. 0 (the default) means no timeout: such jobs
|
||||||
// run to completion however long that takes.
|
// run to completion however long that takes. It is written even when 0 —
|
||||||
DefaultTimeoutSeconds int `json:"default_timeout_seconds,omitempty"`
|
// omitempty would hide a deliberate choice from the hand-editable config.
|
||||||
|
DefaultTimeoutSeconds int `json:"default_timeout_seconds"`
|
||||||
Paused bool `json:"paused,omitempty"`
|
Paused bool `json:"paused,omitempty"`
|
||||||
// Theme selects the visual appearance. Empty is treated as ThemeDefault so
|
// Theme selects the visual appearance. Empty is treated as ThemeDefault so
|
||||||
// configs written before this field existed keep the original look.
|
// configs written before this field existed keep the original look.
|
||||||
Theme Theme `json:"theme,omitempty"`
|
Theme Theme `json:"theme,omitempty"`
|
||||||
|
// JobListView selects the Jobs list density. Empty is treated as
|
||||||
|
// JobListViewDetailed so configs written before this field existed keep the
|
||||||
|
// current three-line rows.
|
||||||
|
JobListView JobListView `json:"job_list_view,omitempty"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// DefaultConfig returns the built-in default settings. It is the config used
|
// DefaultConfig returns the built-in default settings. It is the config used
|
||||||
@@ -78,6 +103,7 @@ func DefaultConfig() Config {
|
|||||||
ExecutionMode: ExecutionModeParallel,
|
ExecutionMode: ExecutionModeParallel,
|
||||||
OverlapPolicy: OverlapPolicySkip,
|
OverlapPolicy: OverlapPolicySkip,
|
||||||
Theme: ThemeDefault,
|
Theme: ThemeDefault,
|
||||||
|
JobListView: JobListViewDetailed,
|
||||||
DefaultTimeoutSeconds: 0,
|
DefaultTimeoutSeconds: 0,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,30 @@
|
|||||||
|
package domain
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
// TestJobListViewIsCompact pins the normalization rule: only the exact
|
||||||
|
// "compact" value selects the one-line rows, so empty and unrecognised values
|
||||||
|
// (including configs written before the field existed) keep the detailed look.
|
||||||
|
func TestJobListViewIsCompact(t *testing.T) {
|
||||||
|
cases := []struct {
|
||||||
|
view JobListView
|
||||||
|
want bool
|
||||||
|
}{
|
||||||
|
{JobListViewCompact, true},
|
||||||
|
{JobListViewDetailed, false},
|
||||||
|
{"", false},
|
||||||
|
{"Compact", false},
|
||||||
|
{"tiny", false},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
if got := tc.view.IsCompact(); got != tc.want {
|
||||||
|
t.Errorf("JobListView(%q).IsCompact() = %v, want %v", tc.view, got, tc.want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestDefaultConfigUsesDetailedJobList(t *testing.T) {
|
||||||
|
if got := DefaultConfig().JobListView; got != JobListViewDetailed {
|
||||||
|
t.Errorf("default JobListView = %q, want %q", got, JobListViewDetailed)
|
||||||
|
}
|
||||||
|
}
|
||||||
+15
-5
@@ -15,9 +15,19 @@ type Job struct {
|
|||||||
StartOnly bool `json:"start_only,omitempty"`
|
StartOnly bool `json:"start_only,omitempty"`
|
||||||
Enabled bool `json:"enabled"`
|
Enabled bool `json:"enabled"`
|
||||||
OverlapPolicy string `json:"overlap_policy,omitempty"`
|
OverlapPolicy string `json:"overlap_policy,omitempty"`
|
||||||
// TimeoutSeconds bounds how long a run may take before it is killed. 0 means
|
// TimeoutSeconds bounds how long a run may take before it is killed. It is a
|
||||||
// "inherit the global Config.DefaultTimeoutSeconds", mirroring OverlapPolicy:
|
// pointer so the three states stay distinguishable on disk: absent (nil)
|
||||||
// normalizeJobs must leave 0 untouched rather than backfilling the default.
|
// means "inherit the global Config.DefaultTimeoutSeconds", mirroring
|
||||||
// The inherited global default may itself be 0, meaning no timeout at all.
|
// OverlapPolicy's empty string; an explicit 0 means "no timeout" and does
|
||||||
TimeoutSeconds int `json:"timeout_seconds,omitempty"`
|
// not inherit; a positive value is the per-job limit in seconds. The
|
||||||
|
// inherited global default may itself be 0, also meaning no timeout.
|
||||||
|
// normalizeJobs must leave nil untouched rather than backfilling a value.
|
||||||
|
TimeoutSeconds *int `json:"timeout_seconds,omitempty"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// TimeoutSecondsPtr returns a pointer suitable for Job.TimeoutSeconds. It exists
|
||||||
|
// because nil (inherit) and an explicit 0 (no timeout) are different states, so
|
||||||
|
// callers cannot just assign an int.
|
||||||
|
func TimeoutSecondsPtr(seconds int) *int {
|
||||||
|
return &seconds
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -102,9 +102,10 @@ func loadOrCreateConfig(paths Paths) (domain.Config, error) {
|
|||||||
if config.OverlapPolicy == "" {
|
if config.OverlapPolicy == "" {
|
||||||
config.OverlapPolicy = domain.OverlapPolicySkip
|
config.OverlapPolicy = domain.OverlapPolicySkip
|
||||||
}
|
}
|
||||||
if config.DefaultTimeoutSeconds <= 0 {
|
// DefaultTimeoutSeconds is deliberately not normalized: 0 is a meaningful
|
||||||
config.DefaultTimeoutSeconds = 30
|
// value ("no timeout"), not a missing one, so backfilling it here would make
|
||||||
}
|
// the setting impossible to persist. Negative values are rejected by
|
||||||
|
// app.validateConfig before they can be saved.
|
||||||
if config.Theme == "" {
|
if config.Theme == "" {
|
||||||
config.Theme = domain.ThemeDefault
|
config.Theme = domain.ThemeDefault
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -174,12 +174,72 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) {
|
|||||||
if got.Theme != domain.ThemeDefault {
|
if got.Theme != domain.ThemeDefault {
|
||||||
t.Errorf("default Theme = %q, want %q", got.Theme, domain.ThemeDefault)
|
t.Errorf("default Theme = %q, want %q", got.Theme, domain.ThemeDefault)
|
||||||
}
|
}
|
||||||
|
if got.JobListView != domain.JobListViewDetailed {
|
||||||
|
t.Errorf("default JobListView = %q, want %q", got.JobListView, domain.JobListViewDetailed)
|
||||||
|
}
|
||||||
// The function must have written the defaults to gosentry.json.
|
// The function must have written the defaults to gosentry.json.
|
||||||
if _, err := os.Stat(paths.ConfigPath); err != nil {
|
if _, err := os.Stat(paths.ConfigPath); err != nil {
|
||||||
t.Errorf("gosentry.json should have been created: %v", err)
|
t.Errorf("gosentry.json should have been created: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestLoadOrCreateConfigKeepsZeroTimeoutOnReload guards the "0 = no timeout"
|
||||||
|
// setting against being normalized away when an existing gosentry.json is read
|
||||||
|
// back. Loading must not treat 0 as a missing value.
|
||||||
|
func TestLoadOrCreateConfigKeepsZeroTimeoutOnReload(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
paths := Paths{
|
||||||
|
AppDir: dir,
|
||||||
|
ConfigPath: filepath.Join(dir, ConfigFileName),
|
||||||
|
}
|
||||||
|
|
||||||
|
// First call writes the defaults (DefaultTimeoutSeconds = 0) to disk.
|
||||||
|
if _, err := loadOrCreateConfig(paths); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
// Second call takes the "file exists" branch, where normalization runs.
|
||||||
|
reloaded, err := loadOrCreateConfig(paths)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if reloaded.DefaultTimeoutSeconds != 0 {
|
||||||
|
t.Errorf("reloaded DefaultTimeoutSeconds = %d, want 0 (no timeout)", reloaded.DefaultTimeoutSeconds)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestJobTimeoutRoundTripsThreeStates pins the on-disk encoding that keeps
|
||||||
|
// "inherit" and "no timeout" distinguishable: nil is omitted entirely, while an
|
||||||
|
// explicit 0 is written and read back as a set value.
|
||||||
|
func TestJobTimeoutRoundTripsThreeStates(t *testing.T) {
|
||||||
|
jobs := []domain.Job{
|
||||||
|
{ID: 1, Name: "Inherit", TimeoutSeconds: nil},
|
||||||
|
{ID: 2, Name: "No timeout", TimeoutSeconds: domain.TimeoutSecondsPtr(0)},
|
||||||
|
{ID: 3, Name: "Own", TimeoutSeconds: domain.TimeoutSecondsPtr(45)},
|
||||||
|
}
|
||||||
|
|
||||||
|
data, err := json.Marshal(domain.JobsFile{Jobs: jobs})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if want := `"timeout_seconds":0`; !strings.Contains(string(data), want) {
|
||||||
|
t.Fatalf("explicit zero timeout should be written as %s:\n%s", want, data)
|
||||||
|
}
|
||||||
|
|
||||||
|
var got domain.JobsFile
|
||||||
|
if err := json.Unmarshal(data, &got); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if got.Jobs[0].TimeoutSeconds != nil {
|
||||||
|
t.Errorf("unset timeout should stay nil, got %d", *got.Jobs[0].TimeoutSeconds)
|
||||||
|
}
|
||||||
|
if got.Jobs[1].TimeoutSeconds == nil || *got.Jobs[1].TimeoutSeconds != 0 {
|
||||||
|
t.Errorf("explicit zero timeout should survive the round trip, got %v", got.Jobs[1].TimeoutSeconds)
|
||||||
|
}
|
||||||
|
if got.Jobs[2].TimeoutSeconds == nil || *got.Jobs[2].TimeoutSeconds != 45 {
|
||||||
|
t.Errorf("per-job timeout should survive the round trip, got %v", got.Jobs[2].TimeoutSeconds)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestJobsJSONDoesNotPersistRuntimeNoise(t *testing.T) {
|
func TestJobsJSONDoesNotPersistRuntimeNoise(t *testing.T) {
|
||||||
// Job carries only durable configuration; runtime state lives in
|
// Job carries only durable configuration; runtime state lives in
|
||||||
// domain.JobRuntime and is never marshalled. This guards against a future
|
// domain.JobRuntime and is never marshalled. This guards against a future
|
||||||
|
|||||||
@@ -55,9 +55,9 @@ func showJobDialog(w fyne.Window, title string, current job, onSave func(job)) {
|
|||||||
}
|
}
|
||||||
overlapSelect.SetSelected(overlapSelected)
|
overlapSelect.SetSelected(overlapSelected)
|
||||||
timeoutEntry := widget.NewEntry()
|
timeoutEntry := widget.NewEntry()
|
||||||
timeoutEntry.SetPlaceHolder("Empty = use global default")
|
timeoutEntry.SetPlaceHolder("Empty = global default, 0 = no timeout")
|
||||||
if current.TimeoutSeconds > 0 {
|
if current.TimeoutSeconds != nil {
|
||||||
timeoutEntry.SetText(strconv.Itoa(current.TimeoutSeconds))
|
timeoutEntry.SetText(strconv.Itoa(*current.TimeoutSeconds))
|
||||||
}
|
}
|
||||||
|
|
||||||
form := dialog.NewForm(
|
form := dialog.NewForm(
|
||||||
@@ -89,16 +89,17 @@ func showJobDialog(w fyne.Window, title string, current job, onSave func(job)) {
|
|||||||
dialog.ShowError(fmt.Errorf("invalid schedule: %w", err), w)
|
dialog.ShowError(fmt.Errorf("invalid schedule: %w", err), w)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
// An empty timeout inherits the global default (0); any entry must be a
|
// An empty timeout inherits the global default (nil); an explicit 0
|
||||||
|
// means "no timeout" and does not inherit; anything else must be a
|
||||||
// positive whole number of seconds.
|
// positive whole number of seconds.
|
||||||
timeoutSeconds := 0
|
var timeoutSeconds *int
|
||||||
if trimmed := strings.TrimSpace(timeoutEntry.Text); trimmed != "" {
|
if trimmed := strings.TrimSpace(timeoutEntry.Text); trimmed != "" {
|
||||||
parsed, err := strconv.Atoi(trimmed)
|
parsed, err := strconv.Atoi(trimmed)
|
||||||
if err != nil || parsed <= 0 {
|
if err != nil || parsed < 0 {
|
||||||
dialog.ShowError(fmt.Errorf("timeout must be a positive number of seconds, or empty to use the global default"), w)
|
dialog.ShowError(fmt.Errorf("timeout must be 0 (no timeout) or a positive number of seconds, or empty to use the global default"), w)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
timeoutSeconds = parsed
|
timeoutSeconds = domain.TimeoutSecondsPtr(parsed)
|
||||||
}
|
}
|
||||||
current.Name = strings.TrimSpace(name.Text)
|
current.Name = strings.TrimSpace(name.Text)
|
||||||
current.Folder = strings.TrimSpace(folderEntry.Text)
|
current.Folder = strings.TrimSpace(folderEntry.Text)
|
||||||
|
|||||||
+68
-4
@@ -70,6 +70,7 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) {
|
|||||||
}
|
}
|
||||||
selectedFolder := allFolders
|
selectedFolder := allFolders
|
||||||
schedulerPaused := svc.Store().Config.Paused
|
schedulerPaused := svc.Store().Config.Paused
|
||||||
|
listView := svc.Store().Config.JobListView
|
||||||
filteredJobs := filteredJobIndexes(jobs, selectedFolder)
|
filteredJobs := filteredJobIndexes(jobs, selectedFolder)
|
||||||
|
|
||||||
dp := newDetailsPanel(job{}, &domain.JobRuntime{}, svc.Store().Config.OverlapPolicy, svc.Store().Config.DefaultTimeoutSeconds)
|
dp := newDetailsPanel(job{}, &domain.JobRuntime{}, svc.Store().Config.OverlapPolicy, svc.Store().Config.DefaultTimeoutSeconds)
|
||||||
@@ -105,17 +106,45 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// applyRowMode expresses the current view mode as visibility on the row's
|
||||||
|
// four labels. widget.List caches the row template's MinSize, and
|
||||||
|
// list.Refresh() re-creates the template and recomputes it, so hiding lines
|
||||||
|
// is what actually shrinks the rows: compactVBoxLayout and the border layout
|
||||||
|
// both skip hidden children when measuring.
|
||||||
|
applyRowMode := func(inlineStatus, meta, status fyne.CanvasObject) {
|
||||||
|
if listView.IsCompact() {
|
||||||
|
inlineStatus.Show()
|
||||||
|
meta.Hide()
|
||||||
|
status.Hide()
|
||||||
|
return
|
||||||
|
}
|
||||||
|
inlineStatus.Hide()
|
||||||
|
meta.Show()
|
||||||
|
status.Show()
|
||||||
|
}
|
||||||
|
|
||||||
list = widget.NewList(
|
list = widget.NewList(
|
||||||
func() int { return len(filteredJobs) },
|
func() int { return len(filteredJobs) },
|
||||||
func() fyne.CanvasObject {
|
func() fyne.CanvasObject {
|
||||||
name := widget.NewLabelWithStyle("Job name", fyne.TextAlignLeading, fyne.TextStyle{Bold: true})
|
name := widget.NewLabelWithStyle("Job name", fyne.TextAlignLeading, fyne.TextStyle{Bold: true})
|
||||||
|
// Truncating stops a long name from pushing the compact row's status
|
||||||
|
// off the right-hand edge. Labels default to TextWrapOff, which grows
|
||||||
|
// the widget to fit instead.
|
||||||
|
name.Truncation = fyne.TextTruncateClip
|
||||||
|
inlineStatus := widget.NewLabel("status")
|
||||||
meta := widget.NewLabel("schedule")
|
meta := widget.NewLabel("schedule")
|
||||||
status := widget.NewLabel("status")
|
status := widget.NewLabel("status")
|
||||||
return container.New(compactVBoxLayout{spacing: jobRowSpacing}, name, meta, status)
|
applyRowMode(inlineStatus, meta, status)
|
||||||
|
nameLine := container.NewBorder(nil, nil, nil, inlineStatus, name)
|
||||||
|
return container.New(compactVBoxLayout{spacing: jobRowSpacing}, nameLine, meta, status)
|
||||||
},
|
},
|
||||||
func(id widget.ListItemID, item fyne.CanvasObject) {
|
func(id widget.ListItemID, item fyne.CanvasObject) {
|
||||||
row := item.(*fyne.Container)
|
row := item.(*fyne.Container)
|
||||||
name := row.Objects[0].(*widget.Label)
|
// NewBorder keeps the center object first and appends the border slots
|
||||||
|
// after it, so nameLine is [name, inlineStatus].
|
||||||
|
nameLine := row.Objects[0].(*fyne.Container)
|
||||||
|
name := nameLine.Objects[0].(*widget.Label)
|
||||||
|
inlineStatus := nameLine.Objects[1].(*widget.Label)
|
||||||
meta := row.Objects[1].(*widget.Label)
|
meta := row.Objects[1].(*widget.Label)
|
||||||
status := row.Objects[2].(*widget.Label)
|
status := row.Objects[2].(*widget.Label)
|
||||||
|
|
||||||
@@ -124,7 +153,12 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) {
|
|||||||
// Keep each row compact: folder, schedule, and command are shown in one
|
// Keep each row compact: folder, schedule, and command are shown in one
|
||||||
// metadata line so the left pane stays useful even with many jobs.
|
// metadata line so the left pane stays useful even with many jobs.
|
||||||
meta.SetText(app.DisplayFolder(current.Folder) + " " + current.Schedule + " " + app.DisplayInvocation(current))
|
meta.SetText(app.DisplayFolder(current.Folder) + " " + current.Schedule + " " + app.DisplayInvocation(current))
|
||||||
status.SetText(app.StatusText(current, runtimes[current.ID]))
|
statusText := app.StatusText(current, runtimes[current.ID])
|
||||||
|
status.SetText(statusText)
|
||||||
|
inlineStatus.SetText(statusText)
|
||||||
|
// A full Refresh reuses rows built under the previous mode, so
|
||||||
|
// visibility cannot be left to the create callback alone.
|
||||||
|
applyRowMode(inlineStatus, meta, status)
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
list.OnSelected = func(id widget.ListItemID) {
|
list.OnSelected = func(id widget.ListItemID) {
|
||||||
@@ -158,6 +192,32 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) {
|
|||||||
})
|
})
|
||||||
folderSelect.SetSelected(selectedFolder)
|
folderSelect.SetSelected(selectedFolder)
|
||||||
|
|
||||||
|
// viewToggleIcon pairs with viewToggleText: both name the action the button
|
||||||
|
// performs, not the state it is in, matching stopAllButton's convention.
|
||||||
|
viewToggleIcon := func(current domain.JobListView) fyne.Resource {
|
||||||
|
if current.IsCompact() {
|
||||||
|
return theme.ViewFullScreenIcon()
|
||||||
|
}
|
||||||
|
return theme.ListIcon()
|
||||||
|
}
|
||||||
|
viewButton := widget.NewButtonWithIcon(viewToggleText(listView), viewToggleIcon(listView), nil)
|
||||||
|
viewButton.OnTapped = func() {
|
||||||
|
next := nextJobListView(listView)
|
||||||
|
listView = next
|
||||||
|
if err := svc.SetJobListView(next); err != nil {
|
||||||
|
// Roll the mode back and leave the button as it was, so the button
|
||||||
|
// never claims a preference that did not reach disk.
|
||||||
|
listView = nextJobListView(next)
|
||||||
|
dialog.ShowError(err, w)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
viewButton.SetText(viewToggleText(listView))
|
||||||
|
viewButton.SetIcon(viewToggleIcon(listView))
|
||||||
|
// Refresh re-creates the row template, which is what recomputes the
|
||||||
|
// cached row height for the new mode. Selection is untouched.
|
||||||
|
list.Refresh()
|
||||||
|
}
|
||||||
|
|
||||||
addButton := widget.NewButtonWithIcon("New job", theme.ContentAddIcon(), func() {
|
addButton := widget.NewButtonWithIcon("New job", theme.ContentAddIcon(), func() {
|
||||||
showJobDialog(w, "New job", job{Schedule: "@every 1m", Command: "echo GoSentry job ran", Enabled: true}, func(saved job) {
|
showJobDialog(w, "New job", job{Schedule: "@every 1m", Command: "echo GoSentry job ran", Enabled: true}, func(saved job) {
|
||||||
created, err := svc.CreateJob(saved)
|
created, err := svc.CreateJob(saved)
|
||||||
@@ -294,7 +354,11 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) {
|
|||||||
|
|
||||||
toolbar := container.NewHBox(addButton, editButton, runButton, pauseButton, deleteButton, layout.NewSpacer())
|
toolbar := container.NewHBox(addButton, editButton, runButton, pauseButton, deleteButton, layout.NewSpacer())
|
||||||
globalControls := container.NewHBox(stopAllButton, schedulerState, layout.NewSpacer())
|
globalControls := container.NewHBox(stopAllButton, schedulerState, layout.NewSpacer())
|
||||||
sidebarHeader := container.NewVBox(globalControls, widget.NewSeparator(), widget.NewLabelWithStyle("Folder", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}), folderSelect, toolbar)
|
// The view toggle sits beside the folder filter: the border layout gives it
|
||||||
|
// its MinSize on the right and lets the select fill the rest, so the header
|
||||||
|
// gains no height.
|
||||||
|
filterRow := container.NewBorder(nil, nil, nil, viewButton, folderSelect)
|
||||||
|
sidebarHeader := container.NewVBox(globalControls, widget.NewSeparator(), widget.NewLabelWithStyle("Folder", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}), filterRow, toolbar)
|
||||||
sidebar := container.NewBorder(sidebarHeader, nil, nil, nil, list)
|
sidebar := container.NewBorder(sidebarHeader, nil, nil, nil, list)
|
||||||
|
|
||||||
fixedSidebar := container.New(minWidthLayout{width: minJobsSidebarWidth}, sidebar)
|
fixedSidebar := container.New(minWidthLayout{width: minJobsSidebarWidth}, sidebar)
|
||||||
|
|||||||
@@ -1,6 +1,10 @@
|
|||||||
package ui
|
package ui
|
||||||
|
|
||||||
import "strings"
|
import (
|
||||||
|
"strings"
|
||||||
|
|
||||||
|
"gitea.mixdep.ru/mix/gosentry/src/domain"
|
||||||
|
)
|
||||||
|
|
||||||
// lastJobLogs returns a fresh slice of the most recent activity entries for the
|
// lastJobLogs returns a fresh slice of the most recent activity entries for the
|
||||||
// "Selected job activity" panel. Logs are stored newest-first (see
|
// "Selected job activity" panel. Logs are stored newest-first (see
|
||||||
@@ -47,6 +51,26 @@ func filterValue(folder string) string {
|
|||||||
return strings.TrimSpace(folder)
|
return strings.TrimSpace(folder)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// nextJobListView returns the mode the view toggle switches to. Anything that
|
||||||
|
// is not compact reads as detailed, so unknown and legacy values flip to
|
||||||
|
// compact just as an explicit "detailed" does.
|
||||||
|
func nextJobListView(current domain.JobListView) domain.JobListView {
|
||||||
|
if current.IsCompact() {
|
||||||
|
return domain.JobListViewDetailed
|
||||||
|
}
|
||||||
|
return domain.JobListViewCompact
|
||||||
|
}
|
||||||
|
|
||||||
|
// viewToggleText labels the view toggle with the action it performs, not the
|
||||||
|
// current state — the same convention as the "Disable auto"/"Enable auto"
|
||||||
|
// button. It also keeps the on-disk strings away from the user.
|
||||||
|
func viewToggleText(current domain.JobListView) string {
|
||||||
|
if current.IsCompact() {
|
||||||
|
return "Detailed"
|
||||||
|
}
|
||||||
|
return "Compact"
|
||||||
|
}
|
||||||
|
|
||||||
func indexOfID(jobs []job, id int) int {
|
func indexOfID(jobs []job, id int) int {
|
||||||
for index, current := range jobs {
|
for index, current := range jobs {
|
||||||
if current.ID == id {
|
if current.ID == id {
|
||||||
|
|||||||
@@ -3,7 +3,12 @@ package ui
|
|||||||
import (
|
import (
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"gitea.mixdep.ru/mix/gosentry/src/app"
|
||||||
"gitea.mixdep.ru/mix/gosentry/src/domain"
|
"gitea.mixdep.ru/mix/gosentry/src/domain"
|
||||||
|
|
||||||
|
"fyne.io/fyne/v2"
|
||||||
|
"fyne.io/fyne/v2/test"
|
||||||
|
"fyne.io/fyne/v2/widget"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestFilterValue(t *testing.T) {
|
func TestFilterValue(t *testing.T) {
|
||||||
@@ -93,3 +98,179 @@ func TestFilteredJobIndexesEmptySlice(t *testing.T) {
|
|||||||
t.Errorf("empty job list should return empty indexes, got %v", got)
|
t.Errorf("empty job list should return empty indexes, got %v", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestNextJobListViewFlipsBothWays(t *testing.T) {
|
||||||
|
cases := []struct {
|
||||||
|
current, want domain.JobListView
|
||||||
|
}{
|
||||||
|
{domain.JobListViewDetailed, domain.JobListViewCompact},
|
||||||
|
{domain.JobListViewCompact, domain.JobListViewDetailed},
|
||||||
|
// Empty and unknown values read as detailed, so they flip to compact.
|
||||||
|
{"", domain.JobListViewCompact},
|
||||||
|
{"tiny", domain.JobListViewCompact},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
if got := nextJobListView(tc.current); got != tc.want {
|
||||||
|
t.Errorf("nextJobListView(%q) = %q, want %q", tc.current, got, tc.want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// findFirst walks a widget tree depth-first and returns the first object the
|
||||||
|
// match function accepts. Tests use it to reach widgets newJobsView builds
|
||||||
|
// internally rather than returning.
|
||||||
|
func findFirst(root fyne.CanvasObject, match func(fyne.CanvasObject) bool) fyne.CanvasObject {
|
||||||
|
if match(root) {
|
||||||
|
return root
|
||||||
|
}
|
||||||
|
container, ok := root.(*fyne.Container)
|
||||||
|
if !ok {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
for _, child := range container.Objects {
|
||||||
|
if found := findFirst(child, match); found != nil {
|
||||||
|
return found
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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.
|
||||||
|
func jobsSidebar(t *testing.T, content fyne.CanvasObject) fyne.CanvasObject {
|
||||||
|
t.Helper()
|
||||||
|
found := findFirst(content, func(o fyne.CanvasObject) bool {
|
||||||
|
wrapper, ok := o.(*fyne.Container)
|
||||||
|
if !ok {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
_, ok = wrapper.Layout.(minWidthLayout)
|
||||||
|
return ok
|
||||||
|
})
|
||||||
|
if found == nil {
|
||||||
|
t.Fatal("jobs view has no fixed-width sidebar")
|
||||||
|
}
|
||||||
|
return found
|
||||||
|
}
|
||||||
|
|
||||||
|
func jobsList(t *testing.T, content fyne.CanvasObject) *widget.List {
|
||||||
|
t.Helper()
|
||||||
|
found := findFirst(jobsSidebar(t, content), func(o fyne.CanvasObject) bool {
|
||||||
|
_, ok := o.(*widget.List)
|
||||||
|
return ok
|
||||||
|
})
|
||||||
|
if found == nil {
|
||||||
|
t.Fatal("jobs sidebar contains no list widget")
|
||||||
|
}
|
||||||
|
return found.(*widget.List)
|
||||||
|
}
|
||||||
|
|
||||||
|
func jobsViewToggle(t *testing.T, content fyne.CanvasObject) *widget.Button {
|
||||||
|
t.Helper()
|
||||||
|
found := findFirst(jobsSidebar(t, content), func(o fyne.CanvasObject) bool {
|
||||||
|
button, ok := o.(*widget.Button)
|
||||||
|
return ok && (button.Text == "Compact" || button.Text == "Detailed")
|
||||||
|
})
|
||||||
|
if found == nil {
|
||||||
|
t.Fatal("jobs sidebar has no view toggle button")
|
||||||
|
}
|
||||||
|
return found.(*widget.Button)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestJobListViewToggleShrinksRowsAndPersists is the end-to-end guard for the
|
||||||
|
// compact view: one tap must shrink the list rows, relabel the button with the
|
||||||
|
// opposite action, and reach the config — and toggling back must undo all
|
||||||
|
// three. Row height is measured through List.CreateItem/UpdateItem because
|
||||||
|
// that is exactly what widget.List caches as the row height.
|
||||||
|
func TestJobListViewToggleShrinksRowsAndPersists(t *testing.T) {
|
||||||
|
testApp := test.NewApp()
|
||||||
|
defer testApp.Quit()
|
||||||
|
w := testApp.NewWindow("test")
|
||||||
|
defer w.Close()
|
||||||
|
|
||||||
|
store := newTestStore(t)
|
||||||
|
jobs := []domain.Job{
|
||||||
|
{ID: 1, Name: "Nightly backup", Folder: "Maintenance", Schedule: "@every 1m", Command: "echo hi", Enabled: true},
|
||||||
|
}
|
||||||
|
svc := app.NewService(store, jobs)
|
||||||
|
defer svc.Stop()
|
||||||
|
|
||||||
|
content, _ := newJobsView(w, svc)
|
||||||
|
w.SetContent(content)
|
||||||
|
|
||||||
|
list := jobsList(t, content)
|
||||||
|
viewButton := jobsViewToggle(t, content)
|
||||||
|
if viewButton.Text != "Compact" {
|
||||||
|
t.Fatalf("a default config should open detailed: button text = %q, want %q", viewButton.Text, "Compact")
|
||||||
|
}
|
||||||
|
|
||||||
|
rowHeight := func() float32 {
|
||||||
|
t.Helper()
|
||||||
|
row := list.CreateItem()
|
||||||
|
list.UpdateItem(0, row)
|
||||||
|
return row.MinSize().Height
|
||||||
|
}
|
||||||
|
detailedHeight := rowHeight()
|
||||||
|
|
||||||
|
test.Tap(viewButton)
|
||||||
|
if store.Config.JobListView != domain.JobListViewCompact {
|
||||||
|
t.Errorf("after tapping, Config.JobListView = %q, want %q", store.Config.JobListView, domain.JobListViewCompact)
|
||||||
|
}
|
||||||
|
if viewButton.Text != "Detailed" {
|
||||||
|
t.Errorf("after tapping, button text = %q, want %q", viewButton.Text, "Detailed")
|
||||||
|
}
|
||||||
|
compactHeight := rowHeight()
|
||||||
|
if compactHeight >= detailedHeight {
|
||||||
|
t.Errorf("compact row height = %v, want less than detailed %v", compactHeight, detailedHeight)
|
||||||
|
}
|
||||||
|
|
||||||
|
test.Tap(viewButton)
|
||||||
|
if store.Config.JobListView != domain.JobListViewDetailed {
|
||||||
|
t.Errorf("after tapping back, Config.JobListView = %q, want %q", store.Config.JobListView, domain.JobListViewDetailed)
|
||||||
|
}
|
||||||
|
if viewButton.Text != "Compact" {
|
||||||
|
t.Errorf("after tapping back, button text = %q, want %q", viewButton.Text, "Compact")
|
||||||
|
}
|
||||||
|
if got := rowHeight(); got != detailedHeight {
|
||||||
|
t.Errorf("row height after switching back = %v, want the original %v", got, detailedHeight)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestJobListViewCompactConfigOpensCompact checks the persisted preference is
|
||||||
|
// honoured at build time, not just after a tap.
|
||||||
|
func TestJobListViewCompactConfigOpensCompact(t *testing.T) {
|
||||||
|
testApp := test.NewApp()
|
||||||
|
defer testApp.Quit()
|
||||||
|
w := testApp.NewWindow("test")
|
||||||
|
defer w.Close()
|
||||||
|
|
||||||
|
store := newTestStore(t)
|
||||||
|
store.Config.JobListView = domain.JobListViewCompact
|
||||||
|
svc := app.NewService(store, nil)
|
||||||
|
defer svc.Stop()
|
||||||
|
|
||||||
|
content, _ := newJobsView(w, svc)
|
||||||
|
w.SetContent(content)
|
||||||
|
|
||||||
|
if got := jobsViewToggle(t, content).Text; got != "Detailed" {
|
||||||
|
t.Errorf("button text for a compact config = %q, want %q", got, "Detailed")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestViewToggleTextNamesTheAction(t *testing.T) {
|
||||||
|
cases := []struct {
|
||||||
|
current domain.JobListView
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
{domain.JobListViewDetailed, "Compact"},
|
||||||
|
{domain.JobListViewCompact, "Detailed"},
|
||||||
|
{"", "Compact"},
|
||||||
|
{"tiny", "Compact"},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
if got := viewToggleText(tc.current); got != tc.want {
|
||||||
|
t.Errorf("viewToggleText(%q) = %q, want %q", tc.current, got, tc.want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -11,10 +11,13 @@ import (
|
|||||||
"fyne.io/fyne/v2/test"
|
"fyne.io/fyne/v2/test"
|
||||||
)
|
)
|
||||||
|
|
||||||
func newTestService(t *testing.T) *app.Service {
|
// newTestStore builds a Store rooted in a temp directory. It is separate from
|
||||||
|
// newTestService so tests that need a non-default Config (or their own jobs)
|
||||||
|
// can adjust it before handing it to app.NewService.
|
||||||
|
func newTestStore(t *testing.T) *storage.Store {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
dir := t.TempDir()
|
dir := t.TempDir()
|
||||||
store := &storage.Store{
|
return &storage.Store{
|
||||||
Paths: storage.Paths{
|
Paths: storage.Paths{
|
||||||
ExecutablePath: filepath.Join(dir, "gosentry"),
|
ExecutablePath: filepath.Join(dir, "gosentry"),
|
||||||
AppDir: dir,
|
AppDir: dir,
|
||||||
@@ -35,7 +38,11 @@ func newTestService(t *testing.T) *app.Service {
|
|||||||
NotifyOnFailure: true,
|
NotifyOnFailure: true,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
return app.NewService(store, nil)
|
}
|
||||||
|
|
||||||
|
func newTestService(t *testing.T) *app.Service {
|
||||||
|
t.Helper()
|
||||||
|
return app.NewService(newTestStore(t), nil)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestMainViewBuilds(t *testing.T) {
|
func TestMainViewBuilds(t *testing.T) {
|
||||||
|
|||||||
Reference in New Issue
Block a user