Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| e05adc1703 |
@@ -1,26 +0,0 @@
|
|||||||
---
|
|
||||||
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,10 +20,6 @@ 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_/
|
||||||
|
|||||||
@@ -1,59 +0,0 @@
|
|||||||
# 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,7 +49,6 @@ 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
|
||||||
|
|||||||
@@ -203,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`, `nextJobListView`, `viewToggleText` |
|
| `jobs_view_helpers.go` | Pure helpers — `filteredJobIndexes`, `folderOptions`, `filterValue`, `indexOfID`, `lastJobLogs` |
|
||||||
|
|||||||
@@ -4,19 +4,6 @@ All notable GoSentry changes are recorded in this file.
|
|||||||
|
|
||||||
## Unreleased
|
## 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.**
|
**Timeouts: 0 now means "no timeout" at both levels.**
|
||||||
|
|
||||||
- The global **Default timeout** in Settings now defaults to `0`, meaning jobs
|
- The global **Default timeout** in Settings now defaults to `0`, meaning jobs
|
||||||
|
|||||||
@@ -0,0 +1,151 @@
|
|||||||
|
# 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.
|
||||||
@@ -1,86 +0,0 @@
|
|||||||
# 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).
|
|
||||||
+1
-21
@@ -1,8 +1,7 @@
|
|||||||
# 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
|
||||||
|
|
||||||
@@ -13,25 +12,6 @@ what a whole-project review looks at, in [REVIEW.md](REVIEW.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.
|
||||||
|
|||||||
@@ -193,27 +193,6 @@ 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.
|
||||||
|
|||||||
@@ -590,63 +590,6 @@ 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)
|
||||||
|
|||||||
@@ -28,26 +28,6 @@ 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
|
||||||
@@ -82,10 +62,6 @@ type Config struct {
|
|||||||
// 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
|
||||||
@@ -103,7 +79,6 @@ func DefaultConfig() Config {
|
|||||||
ExecutionMode: ExecutionModeParallel,
|
ExecutionMode: ExecutionModeParallel,
|
||||||
OverlapPolicy: OverlapPolicySkip,
|
OverlapPolicy: OverlapPolicySkip,
|
||||||
Theme: ThemeDefault,
|
Theme: ThemeDefault,
|
||||||
JobListView: JobListViewDetailed,
|
|
||||||
DefaultTimeoutSeconds: 0,
|
DefaultTimeoutSeconds: 0,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,30 +0,0 @@
|
|||||||
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)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -174,9 +174,6 @@ 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)
|
||||||
|
|||||||
+4
-68
@@ -70,7 +70,6 @@ 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)
|
||||||
@@ -106,45 +105,17 @@ 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")
|
||||||
applyRowMode(inlineStatus, meta, status)
|
return container.New(compactVBoxLayout{spacing: jobRowSpacing}, name, 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)
|
||||||
// NewBorder keeps the center object first and appends the border slots
|
name := row.Objects[0].(*widget.Label)
|
||||||
// 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)
|
||||||
|
|
||||||
@@ -153,12 +124,7 @@ 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))
|
||||||
statusText := app.StatusText(current, runtimes[current.ID])
|
status.SetText(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) {
|
||||||
@@ -192,32 +158,6 @@ 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)
|
||||||
@@ -354,11 +294,7 @@ 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())
|
||||||
// The view toggle sits beside the folder filter: the border layout gives it
|
sidebarHeader := container.NewVBox(globalControls, widget.NewSeparator(), widget.NewLabelWithStyle("Folder", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}), folderSelect, toolbar)
|
||||||
// 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,10 +1,6 @@
|
|||||||
package ui
|
package ui
|
||||||
|
|
||||||
import (
|
import "strings"
|
||||||
"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
|
||||||
@@ -51,26 +47,6 @@ 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,12 +3,7 @@ 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) {
|
||||||
@@ -98,179 +93,3 @@ 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,13 +11,10 @@ import (
|
|||||||
"fyne.io/fyne/v2/test"
|
"fyne.io/fyne/v2/test"
|
||||||
)
|
)
|
||||||
|
|
||||||
// newTestStore builds a Store rooted in a temp directory. It is separate from
|
func newTestService(t *testing.T) *app.Service {
|
||||||
// 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()
|
||||||
return &storage.Store{
|
store := &storage.Store{
|
||||||
Paths: storage.Paths{
|
Paths: storage.Paths{
|
||||||
ExecutablePath: filepath.Join(dir, "gosentry"),
|
ExecutablePath: filepath.Join(dir, "gosentry"),
|
||||||
AppDir: dir,
|
AppDir: dir,
|
||||||
@@ -38,11 +35,7 @@ func newTestStore(t *testing.T) *storage.Store {
|
|||||||
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