From 0e6b3bcecf615b452a63808418cb03becda9e2b4 Mon Sep 17 00:00:00 2001 From: mixeme Date: Fri, 7 Aug 2026 22:22:08 +0300 Subject: [PATCH] refactor: split source files that exceeded the ~300-line ceiling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mechanical moves only — operations, store, history_view, and settings_view are now split along their existing seams so every file stays within the 250+20% guideline. Document the new layout in ARCHITECTURE.md and close the ROADMAP item. Co-authored-by: Cursor --- docs/ARCHITECTURE.md | 43 +++- docs/CHANGELOG.md | 10 +- docs/ROADMAP.md | 49 ----- src/app/operations.go | 358 --------------------------------- src/app/operations_locked.go | 122 +++++++++++ src/app/operations_settings.go | 169 ++++++++++++++++ src/app/operations_validate.go | 91 +++++++++ src/storage/store.go | 187 ----------------- src/storage/store_config.go | 72 +++++++ src/storage/store_jobs.go | 134 ++++++++++++ src/ui/history_view.go | 91 --------- src/ui/history_view_columns.go | 99 +++++++++ src/ui/settings_view.go | 274 +------------------------ src/ui/settings_view_form.go | 288 ++++++++++++++++++++++++++ 14 files changed, 1024 insertions(+), 963 deletions(-) create mode 100644 src/app/operations_locked.go create mode 100644 src/app/operations_settings.go create mode 100644 src/app/operations_validate.go create mode 100644 src/storage/store_config.go create mode 100644 src/storage/store_jobs.go create mode 100644 src/ui/history_view_columns.go create mode 100644 src/ui/settings_view_form.go diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e910495..d453ba9 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -274,8 +274,9 @@ the moment the window opens. ### `jobs_view.go` file structure -The size guideline for a file in this project is ~250 lines. -`src/ui/jobs_view.go` is split across six files along these seams: +The size guideline for a file in this project is ~250 lines; up to 20% over +(~300 lines) is acceptable. `src/ui/jobs_view.go` is split across six files +along these seams: | File | Contents | |------|----------| @@ -299,11 +300,45 @@ list's highlight at the selected job. ### `settings_view.go` file structure -`src/ui/settings_view.go` is split across three files the same way, once its +`src/ui/settings_view.go` is split across four files the same way, once its own size passed the guideline: | File | Contents | |------|----------| -| `settings_view.go` | `settingsView` — field construction, save, load, validate; the Theme label translation helpers | +| `settings_view.go` | `settingsView` — thin entry point; theme label translation helpers | +| `settings_view_form.go` | `buildSettingsForm` — widget construction, save, load, cancel, and defaults handlers | | `settings_view_layout.go` | `newSettingsLayout`, `settingsSection`, `settingsRow` — the two-column arrangement and the button row | | `settings_view_helpers.go` | Pure helpers — `fyneVersion`, `mustParseURL`, `settingsFolderPath`, `openFolder`, `chooseFile`/`chooseJSONFile`, `chooseFolder` (`chooseFile` also backs `job_dialog.go`'s command browser) | + +### `operations.go` file structure + +`src/app/operations.go` is split across four files along the public API, +locked helpers, and pure validation seams: + +| File | Contents | +|------|----------| +| `operations.go` | Job mutators (`CreateJob` … `SetEnabled`); shared constants (`maxJobLogs`, `timestampLayout`, `errJobNotFound`) | +| `operations_settings.go` | Config mutators — `SetGlobalPause`, `SetJobListView`, `ShouldNotifyOnFailure`, `UpdateSettings` | +| `operations_locked.go` | `*Locked` state helpers (`refreshNextRunLocked` … `nextIDLocked`); `prependLog`, `uiRecord` | +| `operations_validate.go` | Pure validators and normalizers — `normalizeJob`, `validateJob`, `hasFileName`, `validateConfig` | + +### `store.go` file structure + +`src/storage/store.go` is split across three files. Path resolution for the +executable directory lives in `paths.go`; config-relative path resolution stays +with the store API: + +| File | Contents | +|------|----------| +| `store.go` | `Store` struct, `OpenStore`, `PeekKeepRunningInTray`, save API, `ResolveConfiguredPath`, `applyConfigPaths`, atomic JSON writes | +| `store_config.go` | `loadOrCreateConfig` — config load, defaults, and migration shims | +| `store_jobs.go` | `LoadJobsFile`, `loadOrCreateJobs`, `normalizeJobs`, sample jobs, platform-specific demo commands | + +### `history_view.go` file structure + +`src/ui/history_view.go` is split across two files: + +| File | Contents | +|------|----------| +| `history_view_columns.go` | Pure column-width helpers — `textWidth` through `historyColumnWidths`, sample sets, `historyContentValues` | +| `history_view.go` | `historyLog`, `historyHeader`, `newHistoryView`, cell text, `newEvent`, `logFileName` | diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6e3a76d..7603c0c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -94,7 +94,8 @@ History and overlap queues, and a Jobs selection that follows the job.** `TestQuoteLeadingWindowsProgramPathPicksEarliestBoundedExtension`), the deliberately-uncovered list covers everything the profile reports at 0%, and the coverage figure records how to read the total rather than the per-package - lines. `ROADMAP.md` — the over-the-guideline table was re-measured. + lines. `ROADMAP.md` — the over-the-guideline table was re-measured; the split + is now landed and the open item removed. - README's scheduler wording caught up with the 0.11.2 rename of "Pause all" to **Disable auto**, and its notification description matches what the app sends. - `STANDARDS.md` records the rules the review settled: no file I/O under @@ -132,6 +133,13 @@ History and overlap queues, and a Jobs selection that follows the job.** 330-line constructor whose dozen closures shared seven mutable locals is now widgets reading one named state object — which is what made the selection fix above a change in one place instead of five. +- Four more source files over the ~300-line ceiling (250 + 20%) were split in + one pass: `operations.go` into job mutators, config mutators + (`operations_settings.go`), `*Locked` helpers, and pure validators; `store.go` into the store API, config load, and jobs load; + `history_view.go` into column-width helpers and the table widget; and + `settings_view.go` into a thin entry point plus `settings_view_form.go` for + field construction and save/load handlers. `run.go` and `service.go` stay + as-is — both are within the ceiling. - `Service.Store()` is replaced by typed `Service.Config()` and `Service.Paths()` accessors that copy under the lock, so the UI no longer reaches into a shared `*storage.Store`. The Jobs pause control is now driven by `refreshView` diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index f11e7d4..5b6f2a1 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -139,55 +139,6 @@ Design notes / open questions: Service exposes import/export operations; the UI only picks the file and shows the outcome. -### Split the files that are over the size guideline - -[ARCHITECTURE.md](ARCHITECTURE.md) sets a ~250-line guideline per source file -and records the `jobs_view.go` and `settings_view.go` splits as the worked -examples. `jobs_view.go` was split again in 1.0.3 — into view, state, list, and -toolbar — because the selection defect it carried was a symptom of the size -(one 330-line constructor over seven shared locals). Six non-test files are -over the guideline: - -| File | Lines | -|------|-------| -| `src/app/operations.go` | 529 | -| `src/storage/store.go` | 382 | -| `src/ui/history_view.go` | 355 | -| `src/ui/settings_view.go` | 326 | -| `src/app/run.go` | 275 | -| `src/app/service.go` | 252 | - -The remaining six are deliberately deferred rather than done piecemeal: a -split touches every reader of the file, and doing them in one pass keeps the -seams consistent instead of settling each one its own way. Splitting is -also the kind of change that reads as pure movement while quietly dropping a -function, so it wants one careful pass, not a hurried one per file. - -Seams visible today, as a starting point rather than a decision: - -- **`operations.go`** — the worst overage and the clearest split: the public - mutating operations (`CreateJob` … `UpdateSettings`), the `…Locked` state - helpers that only they call, and the pure validators and normalizers - (`normalizeJob`, `validateJob`, `hasFileName`, `validateConfig`) are three - distinct jobs already sitting in three consecutive blocks. -- **`history_view.go`** — the column-measuring helpers (`textWidth` through - `historyColumnWidths`) are pure, already unit-tested, and independent of the - table they size. -- **`store.go`** — path resolution, the config load/normalize path, and the jobs - load/normalize path are three separate concerns in one file. -- **`run.go`**, **`settings_view.go`**, **`service.go`** — barely over. Worth - re-measuring at the time; if a pass elsewhere has shrunk them, leave them - alone rather than splitting for the sake of the number. The counts above move - a few lines either way with any edit, so re-measure before acting on them - rather than treating the table as current. - -The `jobs_view.go` pass is the worked example for the rest: the constructor was -broken up along the state it shared, not along line count, and the split landed -with the selection fix rather than promising it separately. - -Scope note: the guideline is about source files. Test files are much larger and -that is fine — a table-driven test file grows with the cases it covers. - ### Window size persistence *(frozen)* Window size is currently **not** saved on quit or close. Saving was disabled diff --git a/src/app/operations.go b/src/app/operations.go index 006b70c..3fcea10 100644 --- a/src/app/operations.go +++ b/src/app/operations.go @@ -3,13 +3,9 @@ package app import ( "errors" "fmt" - "path/filepath" - "strings" "time" "gitea.mixdep.ru/mix/gosentry/src/domain" - "gitea.mixdep.ru/mix/gosentry/src/runner" - "gitea.mixdep.ru/mix/gosentry/src/storage" ) // maxJobLogs bounds the in-memory activity list kept per job. The full history @@ -173,357 +169,3 @@ func (s *Service) SetEnabled(id int, enabled bool) error { s.emit(JobChanged{JobID: id}) return nil } - -// SetGlobalPause flips the global pause that gates scheduled execution. -// Manual "Run now" remains available while paused. Each enabled job's next-run -// text reflects the new state immediately so the list view is understandable -// before the next tick. A "Paused"/"Resumed" scheduler activity record and a -// SchedulerStateChanged event are emitted. -func (s *Service) SetGlobalPause(paused bool) error { - s.mu.Lock() - s.paused = paused - s.store.Config.Paused = paused - now := time.Now() - for index := range s.jobs { - job := &s.jobs[index] - runtime := s.runtimeForLocked(job) - if paused { - // A "queue" backlog counts occurrences missed *while paused is off*; once - // paused, none of those correspond to anything the user would expect - // replayed on resume, so drop it rather than letting a stale counter fire - // a deferred run for an occurrence from before the pause. - runtime.PendingRuns = 0 - } - s.refreshNextRunFromLocked(job, runtime, now) - } - save := s.deferSaveLocked(s.store.PrepareSaveConfig()) - s.mu.Unlock() - - if err := save(); err != nil { - return err - } - state, detail := "Resumed", "All job execution resumed" - if paused { - state, detail = "Paused", "All job execution paused" - } - s.emit(RunRecorded{Record: uiRecord(0, "Scheduler", state, detail)}) - s.emit(SchedulerStateChanged{Paused: paused}) - 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 - save := s.deferSaveLocked(s.store.PrepareSaveConfig()) - s.mu.Unlock() - return save() -} - -// ShouldNotifyOnFailure reports whether the user has enabled desktop -// notifications for failed job runs. It reads the config under mu so it is -// safe to call from any goroutine. -func (s *Service) ShouldNotifyOnFailure() bool { - s.mu.Lock() - defer s.mu.Unlock() - return s.store.Config.NotifyOnFailure -} - -// UpdateSettings validates and persists a new application configuration. The -// loaded jobs are re-saved because the jobs file may have changed, and log -// cleanup runs so a tightened retention policy takes effect immediately. -// -// Pointing the config at a different jobs file that already exists adopts that -// file: its jobs replace the loaded ones, which is the only way the user can -// switch between job lists. A path with no file there yet receives the current -// jobs instead, which is how the jobs file is renamed or relocated. Adoption -// discards all runtime state, so it is refused while a job is running. -func (s *Service) UpdateSettings(config domain.Config) error { - if err := validateConfig(config); err != nil { - return err - } - // The path is stored exactly as it is resolved, so a hand-typed value with - // stray spaces cannot make the saved setting and the file in use disagree. - config.JobsFile = strings.TrimSpace(config.JobsFile) - - s.mu.Lock() - // AppDir is fixed for the process and only UpdateSettings itself — a UI - // action — can move JobsPath, so this snapshot stays valid across the reads - // below. - appDir := s.store.Paths.AppDir - jobsPath := storage.ResolveConfiguredPath(appDir, config.JobsFile) - switching := jobsPath != s.store.Paths.JobsPath - running := s.anyRunningLocked() - s.mu.Unlock() - - if switching && running { - return errors.New("cannot change the jobs file while a job is running") - } - // Read the new file, and reconstruct its jobs' statistics from the logs the - // new config points at, before anything is written and while no lock is held: - // both are file I/O, and SeedStats opens every log in the directory. A file - // that cannot be parsed leaves both the config and the current jobs untouched. - var adopted []domain.Job - var seeds map[int]runner.SeededStats - if switching { - jobs, found, err := storage.LoadJobsFile(jobsPath) - if err != nil { - return fmt.Errorf("read jobs file %s: %w", jobsPath, err) - } - if found { - adopted = jobs - seeds = runner.SeedStats(storage.ResolveConfiguredPath(appDir, config.LogsDir), jobs, config.MaxLogFiles) - } - } - - s.mu.Lock() - // The guard above was evaluated before the reads, off the lock, so re-check - // it: a scheduled run may have started in the meantime, and adoption drops - // every runtime. - if switching && s.anyRunningLocked() { - s.mu.Unlock() - return errors.New("cannot change the jobs file while a job is running") - } - s.store.Config = config - saveConfig := s.store.PrepareSaveConfig() - if adopted != nil { - s.adoptJobsLocked(adopted) - s.applySeededStatsLocked(seeds) - } - // PrepareSaveConfig re-resolved the paths from the new config, so the jobs - // write targets the (possibly new) jobs file and cleanup targets the new logs - // dir. Adopted jobs are written back too, which persists the IDs and defaults - // that normalization filled in, exactly as loading them at startup would. The - // jobs write is skipped when the config write fails, because both writes run - // in the order prepared and stop at the first error. - save := s.deferSaveLocked(saveConfig, s.store.PrepareSaveJobs(s.jobs)) - loaded := len(s.jobs) - logsDir := s.store.Paths.LogsDir - maxFiles := s.store.Config.MaxLogFiles - maxAge := s.store.Config.MaxLogAgeDays - s.mu.Unlock() - - saveErr := save() - if adopted != nil { - // A broad JobChanged redraws the job list; JobsLoaded tells the user in - // History which file those jobs came from, since nothing was asked. Both - // are emitted even when the write failed: the adopted jobs are already the - // in-memory list, and a job list the user cannot see would be worse than - // the error they are about to be shown. - s.emit(JobsLoaded{Path: jobsPath, Count: loaded}) - s.emit(JobChanged{}) - } - if saveErr != nil { - return saveErr - } - return runner.CleanupLogs(logsDir, maxFiles, maxAge) -} - -// refreshNextRunLocked recomputes a job's next-run display from the current time, -// honoring enabled/paused state. The caller must hold mu. -func (s *Service) refreshNextRunLocked(job *domain.Job, runtime *domain.JobRuntime) { - s.refreshNextRunFromLocked(job, runtime, time.Now()) -} - -// refreshNextRunFromLocked is refreshNextRunLocked with an explicit reference -// time, used when one timestamp should drive a whole batch (e.g. a global -// pause). The caller must hold mu. -func (s *Service) refreshNextRunFromLocked(job *domain.Job, runtime *domain.JobRuntime, from time.Time) { - if !job.Enabled { - runtime.NextRun = "Paused" - runtime.NextDue = time.Time{} - return - } - if s.paused { - runtime.NextRun = "Scheduler paused" - runtime.NextDue = time.Time{} - return - } - s.prepareNextRunLocked(job, runtime, from) -} - -// prepareNextRunLocked computes the concrete next-due time from the cached -// schedule. A missing cache entry means the schedule string was unparseable. -// The caller must hold mu. -func (s *Service) prepareNextRunLocked(job *domain.Job, runtime *domain.JobRuntime, from time.Time) { - sched, ok := s.schedules[job.ID] - if !ok { - runtime.NextRun = "Invalid schedule" - runtime.NextDue = time.Time{} - return - } - runtime.NextDue = sched.Next(from) - runtime.NextRun = runtime.NextDue.Format(timestampLayout) -} - -// parseScheduleLocked caches a parsed schedule for the job, dropping the cache -// entry when the schedule string is invalid so prepareNextRunLocked can tell the -// two apart. The caller must hold mu. -func (s *Service) parseScheduleLocked(job *domain.Job) { - sched, err := domain.Parse(job.Schedule) - if err != nil { - delete(s.schedules, job.ID) - return - } - s.schedules[job.ID] = sched -} - -// findByIDLocked returns a pointer into the jobs slice for the job with the -// given ID, or nil. The caller must hold mu. -func (s *Service) findByIDLocked(id int) *domain.Job { - index := s.indexByIDLocked(id) - if index < 0 { - return nil - } - return &s.jobs[index] -} - -// indexByIDLocked returns the slice index of the job with the given ID, or -1. -// The caller must hold mu. -func (s *Service) indexByIDLocked(id int) int { - for index := range s.jobs { - if s.jobs[index].ID == id { - return index - } - } - return -1 -} - -// runtimeForLocked returns the runtime for a job, lazily creating it if missing -// so the Service stays robust if a job lacks an entry. The caller must hold mu. -func (s *Service) runtimeForLocked(job *domain.Job) *domain.JobRuntime { - runtime, ok := s.runtimes[job.ID] - if !ok || runtime == nil { - runtime = domain.NewRuntime(*job) - s.runtimes[job.ID] = runtime - } - return runtime -} - -// nextIDLocked returns the smallest ID greater than every loaded job's ID. The -// caller must hold mu. -func (s *Service) nextIDLocked() int { - next := 1 - for index := range s.jobs { - if s.jobs[index].ID >= next { - next = s.jobs[index].ID + 1 - } - } - return next -} - -// prependLog adds a record to the front of a runtime's activity list and caps -// its length so it cannot grow without bound. -func prependLog(runtime *domain.JobRuntime, record domain.RunRecord) { - runtime.Logs = append([]domain.RunRecord{record}, runtime.Logs...) - if len(runtime.Logs) > maxJobLogs { - runtime.Logs = runtime.Logs[:maxJobLogs] - } -} - -// uiRecord builds an activity record for a user/Service action, using the same -// timestamp shape and "UI" trigger as the GUI did so History stays consistent. -func uiRecord(jobID int, jobName string, state string, detail string) domain.RunRecord { - return domain.RunRecord{ - Time: time.Now().Format(timestampLayout), - JobID: jobID, - JobName: jobName, - Trigger: "UI", - State: state, - Detail: detail, - } -} - -// normalizeJob trims user-entered fields and applies the same defaults the job -// dialog used, so callers do not have to. -func normalizeJob(job *domain.Job) { - job.Name = strings.TrimSpace(job.Name) - job.Folder = strings.TrimSpace(job.Folder) - job.Schedule = strings.TrimSpace(job.Schedule) - job.Command = strings.TrimSpace(job.Command) - job.Arguments = strings.TrimSpace(job.Arguments) -} - -// validateJob enforces the minimum executable definition: name, schedule, and -// command must be present. Folder is optional. The schedule string itself is not -// rejected for being unparseable — that surfaces later as an "Invalid schedule" -// next-run, matching the prior behavior. -func validateJob(job domain.Job) error { - if job.Name == "" || job.Schedule == "" || job.Command == "" { - return errors.New("name, schedule, and command are required") - } - policy := strings.TrimSpace(job.OverlapPolicy) - if policy != "" && policy != string(domain.OverlapPolicySkip) && policy != string(domain.OverlapPolicyQueue) { - return errors.New("overlap policy must be 'skip', 'queue', or empty") - } - if job.TimeoutSeconds != nil && *job.TimeoutSeconds < 0 { - return errors.New("timeout must be zero (no timeout) or a positive number of seconds, or unset to inherit the global default") - } - return nil -} - -// hasFileName reports whether a path ends in something that can be a file name. -// It is a syntax check only — an existing directory whose name looks like a file -// name still passes, and fails at write time — but it catches the shapes a user -// types when they mean a folder: a trailing separator, "." and "..". -func hasFileName(path string) bool { - if strings.HasSuffix(path, "/") || strings.HasSuffix(path, string(filepath.Separator)) { - return false - } - switch filepath.Base(path) { - case ".", "..", string(filepath.Separator): - return false - } - return true -} - -// validateConfig rejects settings that would break persistence or cleanup. -func validateConfig(config domain.Config) error { - jobsFile := strings.TrimSpace(config.JobsFile) - if jobsFile == "" { - return errors.New("jobs file is required") - } - // A path that names only a folder would be written to as if it were a file - // and fail later with an opaque OS error, so require a file name here. - if !hasFileName(jobsFile) { - return errors.New("jobs file must include a file name") - } - if strings.TrimSpace(config.LogsDir) == "" { - return errors.New("logs directory is required") - } - // 0 means "keep everything" (see runner.CleanupLogs); only a negative count - // is rejected, the same three-state shape as DefaultTimeoutSeconds below. - if config.MaxLogFiles < 0 { - return errors.New("max log files must be zero (unlimited) or a positive number") - } - if config.MaxLogAgeDays < 0 { - return errors.New("max log age days must be zero (unlimited) or a positive number") - } - if config.ExecutionMode != domain.ExecutionModeParallel && config.ExecutionMode != domain.ExecutionModeSequential { - return errors.New("execution mode must be 'parallel' or 'sequential'") - } - if config.OverlapPolicy != domain.OverlapPolicySkip && config.OverlapPolicy != domain.OverlapPolicyQueue { - return errors.New("overlap policy must be 'skip' or 'queue'") - } - if config.DefaultTimeoutSeconds < 0 { - return errors.New("default timeout must not be negative (0 means no timeout)") - } - // Empty Theme is accepted and normalized to the branded theme on load, so - // older configs (and hand-built ones) stay valid without an explicit theme. - if config.Theme != "" && config.Theme != domain.ThemeSystem && config.Theme != domain.ThemeGoSentry { - return errors.New("theme must be 'system' or 'gosentry'") - } - return nil -} diff --git a/src/app/operations_locked.go b/src/app/operations_locked.go new file mode 100644 index 0000000..4e04579 --- /dev/null +++ b/src/app/operations_locked.go @@ -0,0 +1,122 @@ +package app + +import ( + "time" + + "gitea.mixdep.ru/mix/gosentry/src/domain" +) + +// refreshNextRunLocked recomputes a job's next-run display from the current time, +// honoring enabled/paused state. The caller must hold mu. +func (s *Service) refreshNextRunLocked(job *domain.Job, runtime *domain.JobRuntime) { + s.refreshNextRunFromLocked(job, runtime, time.Now()) +} + +// refreshNextRunFromLocked is refreshNextRunLocked with an explicit reference +// time, used when one timestamp should drive a whole batch (e.g. a global +// pause). The caller must hold mu. +func (s *Service) refreshNextRunFromLocked(job *domain.Job, runtime *domain.JobRuntime, from time.Time) { + if !job.Enabled { + runtime.NextRun = "Paused" + runtime.NextDue = time.Time{} + return + } + if s.paused { + runtime.NextRun = "Scheduler paused" + runtime.NextDue = time.Time{} + return + } + s.prepareNextRunLocked(job, runtime, from) +} + +// prepareNextRunLocked computes the concrete next-due time from the cached +// schedule. A missing cache entry means the schedule string was unparseable. +// The caller must hold mu. +func (s *Service) prepareNextRunLocked(job *domain.Job, runtime *domain.JobRuntime, from time.Time) { + sched, ok := s.schedules[job.ID] + if !ok { + runtime.NextRun = "Invalid schedule" + runtime.NextDue = time.Time{} + return + } + runtime.NextDue = sched.Next(from) + runtime.NextRun = runtime.NextDue.Format(timestampLayout) +} + +// parseScheduleLocked caches a parsed schedule for the job, dropping the cache +// entry when the schedule string is invalid so prepareNextRunLocked can tell the +// two apart. The caller must hold mu. +func (s *Service) parseScheduleLocked(job *domain.Job) { + sched, err := domain.Parse(job.Schedule) + if err != nil { + delete(s.schedules, job.ID) + return + } + s.schedules[job.ID] = sched +} + +// findByIDLocked returns a pointer into the jobs slice for the job with the +// given ID, or nil. The caller must hold mu. +func (s *Service) findByIDLocked(id int) *domain.Job { + index := s.indexByIDLocked(id) + if index < 0 { + return nil + } + return &s.jobs[index] +} + +// indexByIDLocked returns the slice index of the job with the given ID, or -1. +// The caller must hold mu. +func (s *Service) indexByIDLocked(id int) int { + for index := range s.jobs { + if s.jobs[index].ID == id { + return index + } + } + return -1 +} + +// runtimeForLocked returns the runtime for a job, lazily creating it if missing +// so the Service stays robust if a job lacks an entry. The caller must hold mu. +func (s *Service) runtimeForLocked(job *domain.Job) *domain.JobRuntime { + runtime, ok := s.runtimes[job.ID] + if !ok || runtime == nil { + runtime = domain.NewRuntime(*job) + s.runtimes[job.ID] = runtime + } + return runtime +} + +// nextIDLocked returns the smallest ID greater than every loaded job's ID. The +// caller must hold mu. +func (s *Service) nextIDLocked() int { + next := 1 + for index := range s.jobs { + if s.jobs[index].ID >= next { + next = s.jobs[index].ID + 1 + } + } + return next +} + +// prependLog adds a record to the front of a runtime's activity list and caps +// its length so it cannot grow without bound. +func prependLog(runtime *domain.JobRuntime, record domain.RunRecord) { + runtime.Logs = append([]domain.RunRecord{record}, runtime.Logs...) + if len(runtime.Logs) > maxJobLogs { + runtime.Logs = runtime.Logs[:maxJobLogs] + } +} + +// uiRecord builds an activity record for a user/Service action, using the same +// timestamp shape and "UI" trigger as the GUI did so History stays consistent. +func uiRecord(jobID int, jobName string, state string, detail string) domain.RunRecord { + return domain.RunRecord{ + Time: time.Now().Format(timestampLayout), + JobID: jobID, + JobName: jobName, + Trigger: "UI", + State: state, + Detail: detail, + } +} diff --git a/src/app/operations_settings.go b/src/app/operations_settings.go new file mode 100644 index 0000000..383e1de --- /dev/null +++ b/src/app/operations_settings.go @@ -0,0 +1,169 @@ +package app + +import ( + "errors" + "fmt" + "strings" + "time" + + "gitea.mixdep.ru/mix/gosentry/src/domain" + "gitea.mixdep.ru/mix/gosentry/src/runner" + "gitea.mixdep.ru/mix/gosentry/src/storage" +) + +// SetGlobalPause flips the global pause that gates scheduled execution. +// Manual "Run now" remains available while paused. Each enabled job's next-run +// text reflects the new state immediately so the list view is understandable +// before the next tick. A "Paused"/"Resumed" scheduler activity record and a +// SchedulerStateChanged event are emitted. +func (s *Service) SetGlobalPause(paused bool) error { + s.mu.Lock() + s.paused = paused + s.store.Config.Paused = paused + now := time.Now() + for index := range s.jobs { + job := &s.jobs[index] + runtime := s.runtimeForLocked(job) + if paused { + // A "queue" backlog counts occurrences missed *while paused is off*; once + // paused, none of those correspond to anything the user would expect + // replayed on resume, so drop it rather than letting a stale counter fire + // a deferred run for an occurrence from before the pause. + runtime.PendingRuns = 0 + } + s.refreshNextRunFromLocked(job, runtime, now) + } + save := s.deferSaveLocked(s.store.PrepareSaveConfig()) + s.mu.Unlock() + + if err := save(); err != nil { + return err + } + state, detail := "Resumed", "All job execution resumed" + if paused { + state, detail = "Paused", "All job execution paused" + } + s.emit(RunRecorded{Record: uiRecord(0, "Scheduler", state, detail)}) + s.emit(SchedulerStateChanged{Paused: paused}) + 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 + save := s.deferSaveLocked(s.store.PrepareSaveConfig()) + s.mu.Unlock() + return save() +} + +// ShouldNotifyOnFailure reports whether the user has enabled desktop +// notifications for failed job runs. It reads the config under mu so it is +// safe to call from any goroutine. +func (s *Service) ShouldNotifyOnFailure() bool { + s.mu.Lock() + defer s.mu.Unlock() + return s.store.Config.NotifyOnFailure +} + +// UpdateSettings validates and persists a new application configuration. The +// loaded jobs are re-saved because the jobs file may have changed, and log +// cleanup runs so a tightened retention policy takes effect immediately. +// +// Pointing the config at a different jobs file that already exists adopts that +// file: its jobs replace the loaded ones, which is the only way the user can +// switch between job lists. A path with no file there yet receives the current +// jobs instead, which is how the jobs file is renamed or relocated. Adoption +// discards all runtime state, so it is refused while a job is running. +func (s *Service) UpdateSettings(config domain.Config) error { + if err := validateConfig(config); err != nil { + return err + } + // The path is stored exactly as it is resolved, so a hand-typed value with + // stray spaces cannot make the saved setting and the file in use disagree. + config.JobsFile = strings.TrimSpace(config.JobsFile) + + s.mu.Lock() + // AppDir is fixed for the process and only UpdateSettings itself — a UI + // action — can move JobsPath, so this snapshot stays valid across the reads + // below. + appDir := s.store.Paths.AppDir + jobsPath := storage.ResolveConfiguredPath(appDir, config.JobsFile) + switching := jobsPath != s.store.Paths.JobsPath + running := s.anyRunningLocked() + s.mu.Unlock() + + if switching && running { + return errors.New("cannot change the jobs file while a job is running") + } + // Read the new file, and reconstruct its jobs' statistics from the logs the + // new config points at, before anything is written and while no lock is held: + // both are file I/O, and SeedStats opens every log in the directory. A file + // that cannot be parsed leaves both the config and the current jobs untouched. + var adopted []domain.Job + var seeds map[int]runner.SeededStats + if switching { + jobs, found, err := storage.LoadJobsFile(jobsPath) + if err != nil { + return fmt.Errorf("read jobs file %s: %w", jobsPath, err) + } + if found { + adopted = jobs + seeds = runner.SeedStats(storage.ResolveConfiguredPath(appDir, config.LogsDir), jobs, config.MaxLogFiles) + } + } + + s.mu.Lock() + // The guard above was evaluated before the reads, off the lock, so re-check + // it: a scheduled run may have started in the meantime, and adoption drops + // every runtime. + if switching && s.anyRunningLocked() { + s.mu.Unlock() + return errors.New("cannot change the jobs file while a job is running") + } + s.store.Config = config + saveConfig := s.store.PrepareSaveConfig() + if adopted != nil { + s.adoptJobsLocked(adopted) + s.applySeededStatsLocked(seeds) + } + // PrepareSaveConfig re-resolved the paths from the new config, so the jobs + // write targets the (possibly new) jobs file and cleanup targets the new logs + // dir. Adopted jobs are written back too, which persists the IDs and defaults + // that normalization filled in, exactly as loading them at startup would. The + // jobs write is skipped when the config write fails, because both writes run + // in the order prepared and stop at the first error. + save := s.deferSaveLocked(saveConfig, s.store.PrepareSaveJobs(s.jobs)) + loaded := len(s.jobs) + logsDir := s.store.Paths.LogsDir + maxFiles := s.store.Config.MaxLogFiles + maxAge := s.store.Config.MaxLogAgeDays + s.mu.Unlock() + + saveErr := save() + if adopted != nil { + // A broad JobChanged redraws the job list; JobsLoaded tells the user in + // History which file those jobs came from, since nothing was asked. Both + // are emitted even when the write failed: the adopted jobs are already the + // in-memory list, and a job list the user cannot see would be worse than + // the error they are about to be shown. + s.emit(JobsLoaded{Path: jobsPath, Count: loaded}) + s.emit(JobChanged{}) + } + if saveErr != nil { + return saveErr + } + return runner.CleanupLogs(logsDir, maxFiles, maxAge) +} diff --git a/src/app/operations_validate.go b/src/app/operations_validate.go new file mode 100644 index 0000000..3ed5f90 --- /dev/null +++ b/src/app/operations_validate.go @@ -0,0 +1,91 @@ +package app + +import ( + "errors" + "path/filepath" + "strings" + + "gitea.mixdep.ru/mix/gosentry/src/domain" +) + +// normalizeJob trims user-entered fields and applies the same defaults the job +// dialog used, so callers do not have to. +func normalizeJob(job *domain.Job) { + job.Name = strings.TrimSpace(job.Name) + job.Folder = strings.TrimSpace(job.Folder) + job.Schedule = strings.TrimSpace(job.Schedule) + job.Command = strings.TrimSpace(job.Command) + job.Arguments = strings.TrimSpace(job.Arguments) +} + +// validateJob enforces the minimum executable definition: name, schedule, and +// command must be present. Folder is optional. The schedule string itself is not +// rejected for being unparseable — that surfaces later as an "Invalid schedule" +// next-run, matching the prior behavior. +func validateJob(job domain.Job) error { + if job.Name == "" || job.Schedule == "" || job.Command == "" { + return errors.New("name, schedule, and command are required") + } + policy := strings.TrimSpace(job.OverlapPolicy) + if policy != "" && policy != string(domain.OverlapPolicySkip) && policy != string(domain.OverlapPolicyQueue) { + return errors.New("overlap policy must be 'skip', 'queue', or empty") + } + if job.TimeoutSeconds != nil && *job.TimeoutSeconds < 0 { + return errors.New("timeout must be zero (no timeout) or a positive number of seconds, or unset to inherit the global default") + } + return nil +} + +// hasFileName reports whether a path ends in something that can be a file name. +// It is a syntax check only — an existing directory whose name looks like a file +// name still passes, and fails at write time — but it catches the shapes a user +// types when they mean a folder: a trailing separator, "." and "..". +func hasFileName(path string) bool { + if strings.HasSuffix(path, "/") || strings.HasSuffix(path, string(filepath.Separator)) { + return false + } + switch filepath.Base(path) { + case ".", "..", string(filepath.Separator): + return false + } + return true +} + +// validateConfig rejects settings that would break persistence or cleanup. +func validateConfig(config domain.Config) error { + jobsFile := strings.TrimSpace(config.JobsFile) + if jobsFile == "" { + return errors.New("jobs file is required") + } + // A path that names only a folder would be written to as if it were a file + // and fail later with an opaque OS error, so require a file name here. + if !hasFileName(jobsFile) { + return errors.New("jobs file must include a file name") + } + if strings.TrimSpace(config.LogsDir) == "" { + return errors.New("logs directory is required") + } + // 0 means "keep everything" (see runner.CleanupLogs); only a negative count + // is rejected, the same three-state shape as DefaultTimeoutSeconds below. + if config.MaxLogFiles < 0 { + return errors.New("max log files must be zero (unlimited) or a positive number") + } + if config.MaxLogAgeDays < 0 { + return errors.New("max log age days must be zero (unlimited) or a positive number") + } + if config.ExecutionMode != domain.ExecutionModeParallel && config.ExecutionMode != domain.ExecutionModeSequential { + return errors.New("execution mode must be 'parallel' or 'sequential'") + } + if config.OverlapPolicy != domain.OverlapPolicySkip && config.OverlapPolicy != domain.OverlapPolicyQueue { + return errors.New("overlap policy must be 'skip' or 'queue'") + } + if config.DefaultTimeoutSeconds < 0 { + return errors.New("default timeout must not be negative (0 means no timeout)") + } + // Empty Theme is accepted and normalized to the branded theme on load, so + // older configs (and hand-built ones) stay valid without an explicit theme. + if config.Theme != "" && config.Theme != domain.ThemeSystem && config.Theme != domain.ThemeGoSentry { + return errors.New("theme must be 'system' or 'gosentry'") + } + return nil +} diff --git a/src/storage/store.go b/src/storage/store.go index 24c834b..219a0d9 100644 --- a/src/storage/store.go +++ b/src/storage/store.go @@ -2,11 +2,8 @@ package storage import ( "encoding/json" - "errors" "os" "path/filepath" - "runtime" - "strings" "gitea.mixdep.ru/mix/gosentry/src/domain" ) @@ -115,138 +112,6 @@ func (s *Store) SaveJobs(jobs []domain.Job) error { return s.PrepareSaveJobs(jobs)() } -func loadOrCreateConfig(paths Paths) (domain.Config, error) { - // Defaults favor a portable installation: settings and jobs begin next to the - // executable, while logs are grouped under a dedicated subdirectory. - config := domain.DefaultConfig() - - if _, err := os.Stat(paths.ConfigPath); errors.Is(err, os.ErrNotExist) { - return config, writeJSON(paths.ConfigPath, config) - } - - data, err := os.ReadFile(paths.ConfigPath) - if err != nil { - return domain.Config{}, err - } - // Clearing the default first keeps "the file sets jobs_file" distinguishable - // from "the file omits it", which the jobs_dir migration below depends on. - // The fallbacks restore a value in either case. - config.JobsFile = "" - if err := json.Unmarshal(data, &config); err != nil { - return domain.Config{}, err - } - - // A config written before the setting named a file carries jobs_dir instead - // of jobs_file. Keep its meaning by appending the fixed name that version - // used, then drop the old key so the file is rewritten in the current shape. - if strings.TrimSpace(config.JobsFile) == "" && strings.TrimSpace(config.JobsDir) != "" { - config.JobsFile = filepath.Join(config.JobsDir, JobsFileName) - } - config.JobsDir = "" - if strings.TrimSpace(config.JobsFile) == "" { - // Empty paths are treated as missing values rather than intentional root - // directories. This avoids accidentally writing jobs to unexpected places. - config.JobsFile = JobsFileName - } - if strings.TrimSpace(config.LogsDir) == "" { - config.LogsDir = "logs" - } - // MaxLogFiles and MaxLogAgeDays are deliberately not normalized: 0 means - // "keep everything" (see runner.CleanupLogs), not a missing value, so - // backfilling it here would make that choice impossible to persist. A config - // written before either field existed already carries 0 from json.Unmarshal - // leaving the DefaultConfig() value in config untouched, so old files still - // pick up 100 / 30 without an explicit backfill. - if config.ExecutionMode == "" { - config.ExecutionMode = domain.ExecutionModeParallel - } - if config.OverlapPolicy == "" { - config.OverlapPolicy = domain.OverlapPolicySkip - } - // DefaultTimeoutSeconds is deliberately not normalized: 0 is a meaningful - // 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 == "" { - config.Theme = domain.ThemeGoSentry - } - if config.Theme == "default" { - config.Theme = domain.ThemeSystem - } - return config, nil -} - -// LoadJobsFile reads and normalizes the job definitions at path. The bool -// reports whether the file was there: a missing file is not an error but the -// answer to "is this file already a jobs file?", which is what the Settings tab -// needs when the user points the application at a different jobs file. -func LoadJobsFile(path string) ([]domain.Job, bool, error) { - data, err := os.ReadFile(path) - if errors.Is(err, os.ErrNotExist) { - return nil, false, nil - } - if err != nil { - return nil, false, err - } - var file domain.JobsFile - if err := json.Unmarshal(data, &file); err != nil { - return nil, false, err - } - normalizeJobs(file.Jobs) - return file.Jobs, true, nil -} - -func loadOrCreateJobs(path string) ([]domain.Job, error) { - jobs, found, err := LoadJobsFile(path) - if err != nil { - return nil, err - } - if found { - return jobs, nil - } - // Seed sample jobs so a new user can immediately see scheduled and manual - // execution without inventing a command. The failure sample stays disabled - // so it does not spam notifications; Run now still works for testing. - jobs = defaultJobs() - normalizeJobs(jobs) - return jobs, writeJSON(path, domain.JobsFile{Jobs: jobs}) -} - -func normalizeJobs(jobs []domain.Job) { - next := 1 - seen := make(map[int]bool, len(jobs)) - for index := range jobs { - job := &jobs[index] - if job.ID <= 0 || seen[job.ID] { - // IDs are assigned only when absent or already claimed by an earlier job - // in this file — a hand-edited jobs.json can carry two entries with the - // same ID, which would otherwise share one runtime, one schedule-cache - // entry, and one SeedStats bucket. Existing, unique IDs stay stable - // because History and future log associations use them to identify jobs. - job.ID = next - } - seen[job.ID] = true - if job.ID >= next { - next = job.ID + 1 - } - if strings.TrimSpace(job.Name) == "" { - job.Name = "Untitled job" - } - if strings.TrimSpace(job.Schedule) == "" { - job.Schedule = "@every 1m" - } - if strings.TrimSpace(job.Command) == "" { - // An empty command would fail in a confusing way. A safe echo command - // gives the user something observable and harmless instead. - job.Command = echoCommand("GoSentry job ran") - } - job.Arguments = strings.TrimSpace(job.Arguments) - // Runtime state (last run, next run, status, output, activity) is no longer - // part of Job. It is reconstructed each time the app starts via - // domain.NewRuntime, so normalizeJobs only touches durable configuration. - } -} - // ResolveConfiguredPath turns a file or directory path from the config into the // absolute path the application will actually use. It is exported so callers // outside storage — the settings tab, which opens the configured logs folder — @@ -328,55 +193,3 @@ func writeFileAtomic(dir, path string, data []byte, perm os.FileMode) error { success = true return nil } - -func defaultJobs() []domain.Job { - return []domain.Job{ - { - ID: 1, - Name: "Hello scheduler", - Folder: "Examples", - Schedule: "@every 1m", - Command: echoCommand("GoSentry test job: scheduler is alive"), - Enabled: true, - }, - { - ID: 2, - Name: "Write timestamp", - Folder: "Examples", - Schedule: "*/1 * * * *", - Command: echoCommand("GoSentry test job: timestamp command ran"), - Enabled: true, - }, - { - ID: 3, - Name: "Paused sample", - Schedule: "@every 1m", - Command: echoCommand("This paused sample should not run until enabled"), - Enabled: false, - }, - { - ID: 4, - Name: "Failure notification test", - Folder: "Examples", - Schedule: "@every 1m", - Command: failCommand(), - Enabled: false, - }, - } -} - -func failCommand() string { - if runtime.GOOS == "windows" { - return "exit /b 1" - } - return "exit 1" -} - -func echoCommand(message string) string { - if runtime.GOOS == "windows" { - return "echo " + message - } - // POSIX shells need quotes for messages with spaces. Single quotes inside the - // message are escaped using the standard close-quote/backslash/reopen pattern. - return "echo '" + strings.ReplaceAll(message, "'", "'\\''") + "'" -} diff --git a/src/storage/store_config.go b/src/storage/store_config.go new file mode 100644 index 0000000..a9bc8fb --- /dev/null +++ b/src/storage/store_config.go @@ -0,0 +1,72 @@ +package storage + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + + "gitea.mixdep.ru/mix/gosentry/src/domain" +) + +func loadOrCreateConfig(paths Paths) (domain.Config, error) { + // Defaults favor a portable installation: settings and jobs begin next to the + // executable, while logs are grouped under a dedicated subdirectory. + config := domain.DefaultConfig() + + if _, err := os.Stat(paths.ConfigPath); errors.Is(err, os.ErrNotExist) { + return config, writeJSON(paths.ConfigPath, config) + } + + data, err := os.ReadFile(paths.ConfigPath) + if err != nil { + return domain.Config{}, err + } + // Clearing the default first keeps "the file sets jobs_file" distinguishable + // from "the file omits it", which the jobs_dir migration below depends on. + // The fallbacks restore a value in either case. + config.JobsFile = "" + if err := json.Unmarshal(data, &config); err != nil { + return domain.Config{}, err + } + + // A config written before the setting named a file carries jobs_dir instead + // of jobs_file. Keep its meaning by appending the fixed name that version + // used, then drop the old key so the file is rewritten in the current shape. + if strings.TrimSpace(config.JobsFile) == "" && strings.TrimSpace(config.JobsDir) != "" { + config.JobsFile = filepath.Join(config.JobsDir, JobsFileName) + } + config.JobsDir = "" + if strings.TrimSpace(config.JobsFile) == "" { + // Empty paths are treated as missing values rather than intentional root + // directories. This avoids accidentally writing jobs to unexpected places. + config.JobsFile = JobsFileName + } + if strings.TrimSpace(config.LogsDir) == "" { + config.LogsDir = "logs" + } + // MaxLogFiles and MaxLogAgeDays are deliberately not normalized: 0 means + // "keep everything" (see runner.CleanupLogs), not a missing value, so + // backfilling it here would make that choice impossible to persist. A config + // written before either field existed already carries 0 from json.Unmarshal + // leaving the DefaultConfig() value in config untouched, so old files still + // pick up 100 / 30 without an explicit backfill. + if config.ExecutionMode == "" { + config.ExecutionMode = domain.ExecutionModeParallel + } + if config.OverlapPolicy == "" { + config.OverlapPolicy = domain.OverlapPolicySkip + } + // DefaultTimeoutSeconds is deliberately not normalized: 0 is a meaningful + // 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 == "" { + config.Theme = domain.ThemeGoSentry + } + if config.Theme == "default" { + config.Theme = domain.ThemeSystem + } + return config, nil +} diff --git a/src/storage/store_jobs.go b/src/storage/store_jobs.go new file mode 100644 index 0000000..d2ac9bd --- /dev/null +++ b/src/storage/store_jobs.go @@ -0,0 +1,134 @@ +package storage + +import ( + "encoding/json" + "errors" + "os" + "runtime" + "strings" + + "gitea.mixdep.ru/mix/gosentry/src/domain" +) + +// LoadJobsFile reads and normalizes the job definitions at path. The bool +// reports whether the file was there: a missing file is not an error but the +// answer to "is this file already a jobs file?", which is what the Settings tab +// needs when the user points the application at a different jobs file. +func LoadJobsFile(path string) ([]domain.Job, bool, error) { + data, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return nil, false, nil + } + if err != nil { + return nil, false, err + } + var file domain.JobsFile + if err := json.Unmarshal(data, &file); err != nil { + return nil, false, err + } + normalizeJobs(file.Jobs) + return file.Jobs, true, nil +} + +func loadOrCreateJobs(path string) ([]domain.Job, error) { + jobs, found, err := LoadJobsFile(path) + if err != nil { + return nil, err + } + if found { + return jobs, nil + } + // Seed sample jobs so a new user can immediately see scheduled and manual + // execution without inventing a command. The failure sample stays disabled + // so it does not spam notifications; Run now still works for testing. + jobs = defaultJobs() + normalizeJobs(jobs) + return jobs, writeJSON(path, domain.JobsFile{Jobs: jobs}) +} + +func normalizeJobs(jobs []domain.Job) { + next := 1 + seen := make(map[int]bool, len(jobs)) + for index := range jobs { + job := &jobs[index] + if job.ID <= 0 || seen[job.ID] { + // IDs are assigned only when absent or already claimed by an earlier job + // in this file — a hand-edited jobs.json can carry two entries with the + // same ID, which would otherwise share one runtime, one schedule-cache + // entry, and one SeedStats bucket. Existing, unique IDs stay stable + // because History and future log associations use them to identify jobs. + job.ID = next + } + seen[job.ID] = true + if job.ID >= next { + next = job.ID + 1 + } + if strings.TrimSpace(job.Name) == "" { + job.Name = "Untitled job" + } + if strings.TrimSpace(job.Schedule) == "" { + job.Schedule = "@every 1m" + } + if strings.TrimSpace(job.Command) == "" { + // An empty command would fail in a confusing way. A safe echo command + // gives the user something observable and harmless instead. + job.Command = echoCommand("GoSentry job ran") + } + job.Arguments = strings.TrimSpace(job.Arguments) + // Runtime state (last run, next run, status, output, activity) is no longer + // part of Job. It is reconstructed each time the app starts via + // domain.NewRuntime, so normalizeJobs only touches durable configuration. + } +} + +func defaultJobs() []domain.Job { + return []domain.Job{ + { + ID: 1, + Name: "Hello scheduler", + Folder: "Examples", + Schedule: "@every 1m", + Command: echoCommand("GoSentry test job: scheduler is alive"), + Enabled: true, + }, + { + ID: 2, + Name: "Write timestamp", + Folder: "Examples", + Schedule: "*/1 * * * *", + Command: echoCommand("GoSentry test job: timestamp command ran"), + Enabled: true, + }, + { + ID: 3, + Name: "Paused sample", + Schedule: "@every 1m", + Command: echoCommand("This paused sample should not run until enabled"), + Enabled: false, + }, + { + ID: 4, + Name: "Failure notification test", + Folder: "Examples", + Schedule: "@every 1m", + Command: failCommand(), + Enabled: false, + }, + } +} + +func failCommand() string { + if runtime.GOOS == "windows" { + return "exit /b 1" + } + return "exit 1" +} + +func echoCommand(message string) string { + if runtime.GOOS == "windows" { + return "echo " + message + } + // POSIX shells need quotes for messages with spaces. Single quotes inside the + // message are escaped using the standard close-quote/backslash/reopen pattern. + return "echo '" + strings.ReplaceAll(message, "'", "'\\''") + "'" +} diff --git a/src/ui/history_view.go b/src/ui/history_view.go index aa86058..4fca256 100644 --- a/src/ui/history_view.go +++ b/src/ui/history_view.go @@ -24,87 +24,6 @@ func newEvent(jobID int, jobName string, state string, detail string) event { } } -// textWidth measures how wide s renders at the theme's current body text size. -func textWidth(s string) float32 { - return fyne.MeasureText(s, theme.TextSize(), fyne.TextStyle{}).Width -} - -// cellPadding is the horizontal space a table cell reserves around its text. -// It replaces a hand-tuned pixel constant with the theme's own inner padding -// doubled (one side each), so it follows text size and DPI. -func cellPadding() float32 { return 2 * theme.InnerPadding() } - -// textColumnMinWidth/textColumnMaxWidth bound every content-measured History -// column: the minimum keeps a column readable when its values are short or -// absent, the maximum stops one very long value from dominating the table -// (the table still scrolls horizontally past it). Expressed as measured text -// rather than raw pixels so both follow the theme instead of drifting from it. -func textColumnMinWidth() float32 { return textWidth(strings.Repeat("0", 10)) + cellPadding() } -func textColumnMaxWidth() float32 { return textWidth(strings.Repeat("0", 30)) + cellPadding() } - -// textColumnWidth measures the widest of samples so a table column can be -// sized to fit its content, clamped to [min, max]. Fyne tables do not -// auto-size columns, so without this a fixed width clips values like -// "20260601-100000_SomeJobName.log" in the Log column. -func textColumnWidth(samples []string, min, max float32) float32 { - width := min - for _, text := range samples { - if text == "" { - continue - } - if w := textWidth(text) + cellPadding(); w > width { - width = w - } - } - if width > max { - width = max - } - return width -} - -// historyTriggerSamples is the closed set of Trigger values History ever -// shows (see newEvent and app.operations.go/app.run.go, which produce "UI", -// "Manual" and "Schedule"; historyCellText falls back to "Unknown"). Add a new -// trigger here too if one is introduced there, or the column may clip it. -var historyTriggerSamples = []string{"Schedule", "Manual", "UI", "Unknown"} - -// historyStateSamples is the closed set of State values History ever shows: -// "OK" and "Failed" come from runner.RunJob (runStateDetail/startJobOnly); -// "Started", "Error" and "Jobs loaded" are recorded directly in mainwindow.go. -// Add a new state here too if one is introduced in either place. -var historyStateSamples = []string{"OK", "Failed", "Started", "Error", "Jobs loaded"} - -// historyTimeSample is the rendered form of the timestamp layout every event -// uses (see newEvent), so the Time column needs no content scan: its width is -// fixed by the format string. -const historyTimeSample = "2026-01-02 15:04:05" - -// historyColumnWidths computes every column's width from the current sorted -// rows. Time, Trigger and State are fixed-shape or closed-set columns; Job, -// Detail and Log are free text, so their width tracks the values actually -// present, bounded the same way the Log column always was. -func historyColumnWidths(rows []event) [6]float32 { - var content [3][]string - for i := range content { - content[i] = make([]string, 0, len(rows)) - } - for _, current := range rows { - for i, value := range historyContentValues(current) { - content[i] = append(content[i], value) - } - } - min, max := textColumnMinWidth(), textColumnMaxWidth() - widths := [6]float32{ - 0: textWidth(historyTimeSample) + cellPadding(), - 1: textColumnWidth(historyTriggerSamples, min, max), - 3: textColumnWidth(historyStateSamples, min, max), - } - for i, col := range historyContentCols { - widths[col] = textColumnWidth(content[i], min, max) - } - return widths -} - // maxHistoryRows caps the session History list, the way app.maxJobLogs caps a // job's own activity list. History is never persisted and every record carries // the run's full captured output, so an app left running in the tray — the mode @@ -183,16 +102,6 @@ func (h *historyLog) rescan() { h.widths = historyColumnWidths(h.records) } -// historyContentCols are the columns whose width follows the values actually -// present, in the order historyContentValues returns them. Both the -// incremental fold in add and the full scan in historyColumnWidths go through -// this pair, so they cannot disagree about which columns follow content. -var historyContentCols = [3]int{2, 4, 5} - -func historyContentValues(record event) [3]string { - return [3]string{record.JobName, record.Detail, logFileName(record.LogFile)} -} - // historyHeader is a bold tappable label used in the History table header row. // In Fyne 2.7+ OnSelected is not fired for header cells (Row < 0), so the sort // toggle is wired through the Tappable interface instead. diff --git a/src/ui/history_view_columns.go b/src/ui/history_view_columns.go new file mode 100644 index 0000000..149e04d --- /dev/null +++ b/src/ui/history_view_columns.go @@ -0,0 +1,99 @@ +package ui + +import ( + "strings" + + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/theme" +) + +// textWidth measures how wide s renders at the theme's current body text size. +func textWidth(s string) float32 { + return fyne.MeasureText(s, theme.TextSize(), fyne.TextStyle{}).Width +} + +// cellPadding is the horizontal space a table cell reserves around its text. +// It replaces a hand-tuned pixel constant with the theme's own inner padding +// doubled (one side each), so it follows text size and DPI. +func cellPadding() float32 { return 2 * theme.InnerPadding() } + +// textColumnMinWidth/textColumnMaxWidth bound every content-measured History +// column: the minimum keeps a column readable when its values are short or +// absent, the maximum stops one very long value from dominating the table +// (the table still scrolls horizontally past it). Expressed as measured text +// rather than raw pixels so both follow the theme instead of drifting from it. +func textColumnMinWidth() float32 { return textWidth(strings.Repeat("0", 10)) + cellPadding() } +func textColumnMaxWidth() float32 { return textWidth(strings.Repeat("0", 30)) + cellPadding() } + +// textColumnWidth measures the widest of samples so a table column can be +// sized to fit its content, clamped to [min, max]. Fyne tables do not +// auto-size columns, so without this a fixed width clips values like +// "20260601-100000_SomeJobName.log" in the Log column. +func textColumnWidth(samples []string, min, max float32) float32 { + width := min + for _, text := range samples { + if text == "" { + continue + } + if w := textWidth(text) + cellPadding(); w > width { + width = w + } + } + if width > max { + width = max + } + return width +} + +// historyTriggerSamples is the closed set of Trigger values History ever +// shows (see newEvent and app.operations.go/app.run.go, which produce "UI", +// "Manual" and "Schedule"; historyCellText falls back to "Unknown"). Add a new +// trigger here too if one is introduced there, or the column may clip it. +var historyTriggerSamples = []string{"Schedule", "Manual", "UI", "Unknown"} + +// historyStateSamples is the closed set of State values History ever shows: +// "OK" and "Failed" come from runner.RunJob (runStateDetail/startJobOnly); +// "Started", "Error" and "Jobs loaded" are recorded directly in mainwindow.go. +// Add a new state here too if one is introduced in either place. +var historyStateSamples = []string{"OK", "Failed", "Started", "Error", "Jobs loaded"} + +// historyTimeSample is the rendered form of the timestamp layout every event +// uses (see newEvent), so the Time column needs no content scan: its width is +// fixed by the format string. +const historyTimeSample = "2026-01-02 15:04:05" + +// historyColumnWidths computes every column's width from the current sorted +// rows. Time, Trigger and State are fixed-shape or closed-set columns; Job, +// Detail and Log are free text, so their width tracks the values actually +// present, bounded the same way the Log column always was. +func historyColumnWidths(rows []event) [6]float32 { + var content [3][]string + for i := range content { + content[i] = make([]string, 0, len(rows)) + } + for _, current := range rows { + for i, value := range historyContentValues(current) { + content[i] = append(content[i], value) + } + } + min, max := textColumnMinWidth(), textColumnMaxWidth() + widths := [6]float32{ + 0: textWidth(historyTimeSample) + cellPadding(), + 1: textColumnWidth(historyTriggerSamples, min, max), + 3: textColumnWidth(historyStateSamples, min, max), + } + for i, col := range historyContentCols { + widths[col] = textColumnWidth(content[i], min, max) + } + return widths +} + +// historyContentCols are the columns whose width follows the values actually +// present, in the order historyContentValues returns them. Both the +// incremental fold in add and the full scan in historyColumnWidths go through +// this pair, so they cannot disagree about which columns follow content. +var historyContentCols = [3]int{2, 4, 5} + +func historyContentValues(record event) [3]string { + return [3]string{record.JobName, record.Detail, logFileName(record.LogFile)} +} diff --git a/src/ui/settings_view.go b/src/ui/settings_view.go index 3070add..9f5d8ae 100644 --- a/src/ui/settings_view.go +++ b/src/ui/settings_view.go @@ -1,14 +1,10 @@ package ui import ( - "strconv" - "strings" - "gitea.mixdep.ru/mix/gosentry/src/app" "gitea.mixdep.ru/mix/gosentry/src/domain" "fyne.io/fyne/v2" - "fyne.io/fyne/v2/theme" "fyne.io/fyne/v2/widget" ) @@ -27,275 +23,7 @@ var settingsCaptions = []string{ } func settingsView(w fyne.Window, svc *app.Service, tray *trayState) fyne.CanvasObject { - // saved mirrors the config as last persisted (or freshly loaded at - // construction); it is a local copy the closures below compare the form - // against and reassign after a successful save, rather than holding onto - // the live *storage.Store the Service owns (see app.Service.Config). - // paths never changes after construction of this view — AppDir and - // ConfigPath are fixed for the process — so it is read once, not refreshed. - saved := svc.Config() - paths := svc.Paths() - // updateSaveState compares the form to the saved config and enables Save only - // when something differs. It is defined below (once Save and every field - // exist) but declared here so the field change handlers can reference it. - var updateSaveState func() - // loadFields populates every form control from the given config. It backs - // both the initial load and the Cancel/Defaults buttons below. - var loadFields func(domain.Config) - startOnLogin := widget.NewCheck("Start on login", nil) - startOnLogin.SetChecked(saved.StartOnLogin) - minimizeToTray := widget.NewCheck("Keep running in the system tray", nil) - minimizeToTray.SetChecked(saved.KeepRunningInTray) - autostartStatus := widget.NewLabel("") - trayRestartHint := widget.NewLabel("") - trayRestartHint.Truncation = fyne.TextTruncateClip - // autostartCheckGen guards against an in-flight check's result landing after - // a newer one started (e.g. the user toggles a checkbox again before the - // first check's PowerShell call returns). Both the increment and the compare - // happen on the main/Fyne thread, so this needs no lock of its own. - var autostartCheckGen int - refreshAutostartStatus := func() { - if settingsPendingAutostart(startOnLogin, minimizeToTray, saved) { - autostartStatus.SetText("Pending: save settings to apply") - return - } - // svc.AutostartStatus() reaches readShortcut on Windows, which spawns - // powershell.exe and blocks on CombinedOutput() — hundreds of milliseconds - // of cold start. Running it off the main thread keeps that from freezing - // the window on construction and on every checkbox toggle. - autostartStatus.SetText("Checking...") - autostartCheckGen++ - gen := autostartCheckGen - go func() { - ok, message := svc.AutostartStatus() - fyne.Do(func() { - if gen != autostartCheckGen { - return - } - if ok { - autostartStatus.SetText("OK: " + message) - return - } - autostartStatus.SetText("Problem: " + message) - }) - }() - } - refreshTrayRestartHint := func(pending bool) { - if pending { - trayRestartHint.SetText("Pending: restart GoSentry after save for the tray icon change to take effect.") - return - } - trayRestartHint.SetText("") - } - startOnLogin.OnChanged = func(bool) { - refreshAutostartStatus() - updateSaveState() - } - minimizeToTray.OnChanged = func(bool) { - refreshAutostartStatus() - refreshTrayRestartHint(minimizeToTray.Checked != saved.KeepRunningInTray) - updateSaveState() - } - refreshAutostartStatus() - notifications := widget.NewCheck("Show desktop notifications for failed jobs", nil) - notifications.SetChecked(saved.NotifyOnFailure) - notifications.OnChanged = func(bool) { updateSaveState() } - themeSelect := widget.NewSelect([]string{themeLabelSystem, themeLabelGoSentry}, nil) - themeSelect.SetSelected(themeLabel(saved.Theme)) - // Preview the theme the moment it is picked so the choice is visible before - // saving; Save persists it. Reverting the selection reverts the preview, and - // closing without saving falls back to the stored theme on next launch. - themeSelect.OnChanged = func(string) { - applyTheme(fyne.CurrentApp(), themeFromLabel(themeSelect.Selected)) - updateSaveState() - } - executionModeSelect := widget.NewSelect( - []string{string(domain.ExecutionModeParallel), string(domain.ExecutionModeSequential)}, - nil, - ) - executionModeSelect.SetSelected(string(saved.ExecutionMode)) - executionModeSelect.OnChanged = func(string) { updateSaveState() } - overlapPolicySelect := widget.NewSelect( - []string{string(domain.OverlapPolicySkip), string(domain.OverlapPolicyQueue)}, - nil, - ) - overlapPolicySelect.SetSelected(string(saved.OverlapPolicy)) - overlapPolicySelect.OnChanged = func(string) { updateSaveState() } - defaultTimeout := widget.NewEntry() - defaultTimeout.SetPlaceHolder("0 = no timeout") - defaultTimeout.SetText(strconv.Itoa(saved.DefaultTimeoutSeconds)) - defaultTimeout.OnChanged = func(string) { updateSaveState() } - jobsFile := widget.NewEntry() - jobsFile.SetText(saved.JobsFile) - jobsFile.OnChanged = func(string) { updateSaveState() } - // The picker only offers existing files; a jobs file that does not exist yet - // is entered by typing its path, which Save then creates. - jobsFileBrowse := widget.NewButtonWithIcon("Browse", theme.FileIcon(), func() { - chooseJSONFile(w, jobsFile) - }) - logsDir := widget.NewEntry() - logsDir.SetText(saved.LogsDir) - logsDir.OnChanged = func(string) { updateSaveState() } - logsDirBrowse := widget.NewButtonWithIcon("Browse", theme.FolderOpenIcon(), func() { - chooseFolder(w, logsDir) - }) - // Log files are read outside the app, so the folder gets a direct shortcut - // beside its path instead of making the user copy the path into a file - // manager. It reveals whatever the field currently holds, so an edit can be - // checked before Save. - logsDirOpen := widget.NewButtonWithIcon("Open", theme.FolderIcon(), func() { - openFolder(w, settingsFolderPath(paths.AppDir, logsDir.Text)) - }) - maxLogFiles := widget.NewEntry() - maxLogFiles.SetPlaceHolder("0 = unlimited") - maxLogFiles.SetText(strconv.Itoa(saved.MaxLogFiles)) - maxLogFiles.OnChanged = func(string) { updateSaveState() } - maxLogAgeDays := widget.NewEntry() - maxLogAgeDays.SetPlaceHolder("0 = unlimited") - maxLogAgeDays.SetText(strconv.Itoa(saved.MaxLogAgeDays)) - maxLogAgeDays.OnChanged = func(string) { updateSaveState() } - // Autostart status sits on its own row beneath the checkbox (rather than - // beside it) so the Application section fits within a half-width column. - // Truncating keeps a long status message from forcing the column wider. - autostartStatus.Truncation = fyne.TextTruncateClip - settingsStatus := widget.NewLabel("") - - saveSettings := widget.NewButtonWithIcon("Save settings", theme.DocumentSaveIcon(), func() { - // Only the parse itself happens here: a numeric field has to become an int - // before it can go into a domain.Config at all. Everything else — required - // fields, negative numbers, valid enum values — is Service.UpdateSettings' - // job (see app.validateConfig), so its error is what the user sees rather - // than a second copy of the same rules with different wording. - files, filesErr := strconv.Atoi(strings.TrimSpace(maxLogFiles.Text)) - days, daysErr := strconv.Atoi(strings.TrimSpace(maxLogAgeDays.Text)) - timeout, timeoutErr := strconv.Atoi(strings.TrimSpace(defaultTimeout.Text)) - if filesErr != nil || daysErr != nil || timeoutErr != nil { - settingsStatus.SetText("Max log files, max log age days, and default timeout must be numbers") - return - } - // Build the new config from the form and hand it to the Service, which - // validates it, persists config and jobs to the (possibly new) directory, - // and runs log cleanup so tightened retention limits take effect at once. - config := saved - config.JobsFile = strings.TrimSpace(jobsFile.Text) - config.LogsDir = strings.TrimSpace(logsDir.Text) - config.MaxLogFiles = files - config.MaxLogAgeDays = days - config.StartOnLogin = startOnLogin.Checked - config.KeepRunningInTray = minimizeToTray.Checked - config.NotifyOnFailure = notifications.Checked - config.ExecutionMode = domain.ExecutionMode(executionModeSelect.Selected) - config.OverlapPolicy = domain.OverlapPolicy(overlapPolicySelect.Selected) - config.DefaultTimeoutSeconds = timeout - config.Theme = themeFromLabel(themeSelect.Selected) - previousKeepInTray := saved.KeepRunningInTray - if err := svc.UpdateSettings(config); err != nil { - settingsStatus.SetText("Save failed: " + err.Error()) - return - } - // UpdateSettings may re-resolve paths (a jobs-file switch adopts a - // different directory), so pick up the fresh copy rather than assuming - // config is exactly what landed. - saved = svc.Config() - paths = svc.Paths() - if err := svc.ApplyAutostart(); err != nil { - refreshAutostartStatus() - settingsStatus.SetText("Saved, autostart failed: " + err.Error()) - return - } - refreshAutostartStatus() - tray.apply(fyne.CurrentApp(), w, config.KeepRunningInTray, true) - if previousKeepInTray != config.KeepRunningInTray { - trayRestartHint.SetText(trayRestartHintText) - } else { - refreshTrayRestartHint(false) - } - settingsStatus.SetText("Saved") - // The form now matches the persisted config, so disable Save again. - updateSaveState() - }) - - // Save stays disabled until a field differs from the saved config, so the - // button only invites a click when there is something to persist. The numeric - // fields compare against their canonical string form; any unparsable text - // counts as a change so the user can click Save and see the validation error. - updateSaveState = func() { - c := saved - changed := startOnLogin.Checked != c.StartOnLogin || - minimizeToTray.Checked != c.KeepRunningInTray || - notifications.Checked != c.NotifyOnFailure || - executionModeSelect.Selected != string(c.ExecutionMode) || - overlapPolicySelect.Selected != string(c.OverlapPolicy) || - strings.TrimSpace(defaultTimeout.Text) != strconv.Itoa(c.DefaultTimeoutSeconds) || - strings.TrimSpace(jobsFile.Text) != c.JobsFile || - strings.TrimSpace(logsDir.Text) != c.LogsDir || - strings.TrimSpace(maxLogFiles.Text) != strconv.Itoa(c.MaxLogFiles) || - strings.TrimSpace(maxLogAgeDays.Text) != strconv.Itoa(c.MaxLogAgeDays) || - themeSelect.Selected != themeLabel(c.Theme) - if changed { - saveSettings.Enable() - } else { - saveSettings.Disable() - } - } - updateSaveState() - - // loadFields populates every form control from a config without saving it, - // backing both the Cancel button (reload the saved config, discarding edits) - // and the Defaults button (load the built-in defaults for review before - // Save is clicked). - loadFields = func(c domain.Config) { - startOnLogin.SetChecked(c.StartOnLogin) - minimizeToTray.SetChecked(c.KeepRunningInTray) - notifications.SetChecked(c.NotifyOnFailure) - themeSelect.SetSelected(themeLabel(c.Theme)) - applyTheme(fyne.CurrentApp(), themeFromLabel(themeSelect.Selected)) - executionModeSelect.SetSelected(string(c.ExecutionMode)) - overlapPolicySelect.SetSelected(string(c.OverlapPolicy)) - defaultTimeout.SetText(strconv.Itoa(c.DefaultTimeoutSeconds)) - jobsFile.SetText(c.JobsFile) - logsDir.SetText(c.LogsDir) - maxLogFiles.SetText(strconv.Itoa(c.MaxLogFiles)) - maxLogAgeDays.SetText(strconv.Itoa(c.MaxLogAgeDays)) - if settingsPendingAutostart(startOnLogin, minimizeToTray, saved) { - autostartStatus.SetText("Pending: save settings to apply") - } else { - refreshAutostartStatus() - } - refreshTrayRestartHint(minimizeToTray.Checked != saved.KeepRunningInTray) - settingsStatus.SetText("") - updateSaveState() - } - cancelSettings := widget.NewButtonWithIcon("Cancel", theme.CancelIcon(), func() { - loadFields(saved) - }) - restoreDefaults := widget.NewButtonWithIcon("Defaults", theme.MediaReplayIcon(), func() { - loadFields(domain.DefaultConfig()) - }) - - return newSettingsLayout(settingsFormFields{ - startOnLogin: startOnLogin, - autostartStatus: autostartStatus, - minimizeToTray: minimizeToTray, - trayRestartHint: trayRestartHint, - notifications: notifications, - themeSelect: themeSelect, - executionModeSelect: executionModeSelect, - overlapPolicySelect: overlapPolicySelect, - defaultTimeout: defaultTimeout, - configPath: paths.ConfigPath, - jobsFile: jobsFile, - jobsFileBrowse: jobsFileBrowse, - logsDir: logsDir, - logsDirOpen: logsDirOpen, - logsDirBrowse: logsDirBrowse, - maxLogFiles: maxLogFiles, - maxLogAgeDays: maxLogAgeDays, - saveSettings: saveSettings, - cancelSettings: cancelSettings, - restoreDefaults: restoreDefaults, - settingsStatus: settingsStatus, - }) + return newSettingsLayout(buildSettingsForm(w, svc, tray)) } func settingsPendingAutostart(startOnLogin, minimizeToTray *widget.Check, saved domain.Config) bool { diff --git a/src/ui/settings_view_form.go b/src/ui/settings_view_form.go new file mode 100644 index 0000000..c7aa56b --- /dev/null +++ b/src/ui/settings_view_form.go @@ -0,0 +1,288 @@ +package ui + +import ( + "strconv" + "strings" + + "gitea.mixdep.ru/mix/gosentry/src/app" + "gitea.mixdep.ru/mix/gosentry/src/domain" + + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/theme" + "fyne.io/fyne/v2/widget" +) + +// buildSettingsForm constructs every Settings tab widget and wires save, load, +// and cancel handlers. settingsView delegates here so the constructor file stays +// focused on the thin entry point and theme label helpers. +func buildSettingsForm(w fyne.Window, svc *app.Service, tray *trayState) settingsFormFields { + // saved mirrors the config as last persisted (or freshly loaded at + // construction); it is a local copy the closures below compare the form + // against and reassign after a successful save, rather than holding onto + // the live *storage.Store the Service owns (see app.Service.Config). + // paths never changes after construction of this view — AppDir and + // ConfigPath are fixed for the process — so it is read once, not refreshed. + saved := svc.Config() + paths := svc.Paths() + // updateSaveState compares the form to the saved config and enables Save only + // when something differs. It is defined below (once Save and every field + // exist) but declared here so the field change handlers can reference it. + var updateSaveState func() + // loadFields populates every form control from the given config. It backs + // both the initial load and the Cancel/Defaults buttons below. + var loadFields func(domain.Config) + startOnLogin := widget.NewCheck("Start on login", nil) + startOnLogin.SetChecked(saved.StartOnLogin) + minimizeToTray := widget.NewCheck("Keep running in the system tray", nil) + minimizeToTray.SetChecked(saved.KeepRunningInTray) + autostartStatus := widget.NewLabel("") + trayRestartHint := widget.NewLabel("") + trayRestartHint.Truncation = fyne.TextTruncateClip + // autostartCheckGen guards against an in-flight check's result landing after + // a newer one started (e.g. the user toggles a checkbox again before the + // first check's PowerShell call returns). Both the increment and the compare + // happen on the main/Fyne thread, so this needs no lock of its own. + var autostartCheckGen int + refreshAutostartStatus := func() { + if settingsPendingAutostart(startOnLogin, minimizeToTray, saved) { + autostartStatus.SetText("Pending: save settings to apply") + return + } + // svc.AutostartStatus() reaches readShortcut on Windows, which spawns + // powershell.exe and blocks on CombinedOutput() — hundreds of milliseconds + // of cold start. Running it off the main thread keeps that from freezing + // the window on construction and on every checkbox toggle. + autostartStatus.SetText("Checking...") + autostartCheckGen++ + gen := autostartCheckGen + go func() { + ok, message := svc.AutostartStatus() + fyne.Do(func() { + if gen != autostartCheckGen { + return + } + if ok { + autostartStatus.SetText("OK: " + message) + return + } + autostartStatus.SetText("Problem: " + message) + }) + }() + } + refreshTrayRestartHint := func(pending bool) { + if pending { + trayRestartHint.SetText("Pending: restart GoSentry after save for the tray icon change to take effect.") + return + } + trayRestartHint.SetText("") + } + startOnLogin.OnChanged = func(bool) { + refreshAutostartStatus() + updateSaveState() + } + minimizeToTray.OnChanged = func(bool) { + refreshAutostartStatus() + refreshTrayRestartHint(minimizeToTray.Checked != saved.KeepRunningInTray) + updateSaveState() + } + refreshAutostartStatus() + notifications := widget.NewCheck("Show desktop notifications for failed jobs", nil) + notifications.SetChecked(saved.NotifyOnFailure) + notifications.OnChanged = func(bool) { updateSaveState() } + themeSelect := widget.NewSelect([]string{themeLabelSystem, themeLabelGoSentry}, nil) + themeSelect.SetSelected(themeLabel(saved.Theme)) + // Preview the theme the moment it is picked so the choice is visible before + // saving; Save persists it. Reverting the selection reverts the preview, and + // closing without saving falls back to the stored theme on next launch. + themeSelect.OnChanged = func(string) { + applyTheme(fyne.CurrentApp(), themeFromLabel(themeSelect.Selected)) + updateSaveState() + } + executionModeSelect := widget.NewSelect( + []string{string(domain.ExecutionModeParallel), string(domain.ExecutionModeSequential)}, + nil, + ) + executionModeSelect.SetSelected(string(saved.ExecutionMode)) + executionModeSelect.OnChanged = func(string) { updateSaveState() } + overlapPolicySelect := widget.NewSelect( + []string{string(domain.OverlapPolicySkip), string(domain.OverlapPolicyQueue)}, + nil, + ) + overlapPolicySelect.SetSelected(string(saved.OverlapPolicy)) + overlapPolicySelect.OnChanged = func(string) { updateSaveState() } + defaultTimeout := widget.NewEntry() + defaultTimeout.SetPlaceHolder("0 = no timeout") + defaultTimeout.SetText(strconv.Itoa(saved.DefaultTimeoutSeconds)) + defaultTimeout.OnChanged = func(string) { updateSaveState() } + jobsFile := widget.NewEntry() + jobsFile.SetText(saved.JobsFile) + jobsFile.OnChanged = func(string) { updateSaveState() } + // The picker only offers existing files; a jobs file that does not exist yet + // is entered by typing its path, which Save then creates. + jobsFileBrowse := widget.NewButtonWithIcon("Browse", theme.FileIcon(), func() { + chooseJSONFile(w, jobsFile) + }) + logsDir := widget.NewEntry() + logsDir.SetText(saved.LogsDir) + logsDir.OnChanged = func(string) { updateSaveState() } + logsDirBrowse := widget.NewButtonWithIcon("Browse", theme.FolderOpenIcon(), func() { + chooseFolder(w, logsDir) + }) + // Log files are read outside the app, so the folder gets a direct shortcut + // beside its path instead of making the user copy the path into a file + // manager. It reveals whatever the field currently holds, so an edit can be + // checked before Save. + logsDirOpen := widget.NewButtonWithIcon("Open", theme.FolderIcon(), func() { + openFolder(w, settingsFolderPath(paths.AppDir, logsDir.Text)) + }) + maxLogFiles := widget.NewEntry() + maxLogFiles.SetPlaceHolder("0 = unlimited") + maxLogFiles.SetText(strconv.Itoa(saved.MaxLogFiles)) + maxLogFiles.OnChanged = func(string) { updateSaveState() } + maxLogAgeDays := widget.NewEntry() + maxLogAgeDays.SetPlaceHolder("0 = unlimited") + maxLogAgeDays.SetText(strconv.Itoa(saved.MaxLogAgeDays)) + maxLogAgeDays.OnChanged = func(string) { updateSaveState() } + // Autostart status sits on its own row beneath the checkbox (rather than + // beside it) so the Application section fits within a half-width column. + // Truncating keeps a long status message from forcing the column wider. + autostartStatus.Truncation = fyne.TextTruncateClip + settingsStatus := widget.NewLabel("") + + saveSettings := widget.NewButtonWithIcon("Save settings", theme.DocumentSaveIcon(), func() { + // Only the parse itself happens here: a numeric field has to become an int + // before it can go into a domain.Config at all. Everything else — required + // fields, negative numbers, valid enum values — is Service.UpdateSettings' + // job (see app.validateConfig), so its error is what the user sees rather + // than a second copy of the same rules with different wording. + files, filesErr := strconv.Atoi(strings.TrimSpace(maxLogFiles.Text)) + days, daysErr := strconv.Atoi(strings.TrimSpace(maxLogAgeDays.Text)) + timeout, timeoutErr := strconv.Atoi(strings.TrimSpace(defaultTimeout.Text)) + if filesErr != nil || daysErr != nil || timeoutErr != nil { + settingsStatus.SetText("Max log files, max log age days, and default timeout must be numbers") + return + } + // Build the new config from the form and hand it to the Service, which + // validates it, persists config and jobs to the (possibly new) directory, + // and runs log cleanup so tightened retention limits take effect at once. + config := saved + config.JobsFile = strings.TrimSpace(jobsFile.Text) + config.LogsDir = strings.TrimSpace(logsDir.Text) + config.MaxLogFiles = files + config.MaxLogAgeDays = days + config.StartOnLogin = startOnLogin.Checked + config.KeepRunningInTray = minimizeToTray.Checked + config.NotifyOnFailure = notifications.Checked + config.ExecutionMode = domain.ExecutionMode(executionModeSelect.Selected) + config.OverlapPolicy = domain.OverlapPolicy(overlapPolicySelect.Selected) + config.DefaultTimeoutSeconds = timeout + config.Theme = themeFromLabel(themeSelect.Selected) + previousKeepInTray := saved.KeepRunningInTray + if err := svc.UpdateSettings(config); err != nil { + settingsStatus.SetText("Save failed: " + err.Error()) + return + } + // UpdateSettings may re-resolve paths (a jobs-file switch adopts a + // different directory), so pick up the fresh copy rather than assuming + // config is exactly what landed. + saved = svc.Config() + paths = svc.Paths() + if err := svc.ApplyAutostart(); err != nil { + refreshAutostartStatus() + settingsStatus.SetText("Saved, autostart failed: " + err.Error()) + return + } + refreshAutostartStatus() + tray.apply(fyne.CurrentApp(), w, config.KeepRunningInTray, true) + if previousKeepInTray != config.KeepRunningInTray { + trayRestartHint.SetText(trayRestartHintText) + } else { + refreshTrayRestartHint(false) + } + settingsStatus.SetText("Saved") + // The form now matches the persisted config, so disable Save again. + updateSaveState() + }) + + // Save stays disabled until a field differs from the saved config, so the + // button only invites a click when there is something to persist. The numeric + // fields compare against their canonical string form; any unparsable text + // counts as a change so the user can click Save and see the validation error. + updateSaveState = func() { + c := saved + changed := startOnLogin.Checked != c.StartOnLogin || + minimizeToTray.Checked != c.KeepRunningInTray || + notifications.Checked != c.NotifyOnFailure || + executionModeSelect.Selected != string(c.ExecutionMode) || + overlapPolicySelect.Selected != string(c.OverlapPolicy) || + strings.TrimSpace(defaultTimeout.Text) != strconv.Itoa(c.DefaultTimeoutSeconds) || + strings.TrimSpace(jobsFile.Text) != c.JobsFile || + strings.TrimSpace(logsDir.Text) != c.LogsDir || + strings.TrimSpace(maxLogFiles.Text) != strconv.Itoa(c.MaxLogFiles) || + strings.TrimSpace(maxLogAgeDays.Text) != strconv.Itoa(c.MaxLogAgeDays) || + themeSelect.Selected != themeLabel(c.Theme) + if changed { + saveSettings.Enable() + } else { + saveSettings.Disable() + } + } + updateSaveState() + + // loadFields populates every form control from a config without saving it, + // backing both the Cancel button (reload the saved config, discarding edits) + // and the Defaults button (load the built-in defaults for review before + // Save is clicked). + loadFields = func(c domain.Config) { + startOnLogin.SetChecked(c.StartOnLogin) + minimizeToTray.SetChecked(c.KeepRunningInTray) + notifications.SetChecked(c.NotifyOnFailure) + themeSelect.SetSelected(themeLabel(c.Theme)) + applyTheme(fyne.CurrentApp(), themeFromLabel(themeSelect.Selected)) + executionModeSelect.SetSelected(string(c.ExecutionMode)) + overlapPolicySelect.SetSelected(string(c.OverlapPolicy)) + defaultTimeout.SetText(strconv.Itoa(c.DefaultTimeoutSeconds)) + jobsFile.SetText(c.JobsFile) + logsDir.SetText(c.LogsDir) + maxLogFiles.SetText(strconv.Itoa(c.MaxLogFiles)) + maxLogAgeDays.SetText(strconv.Itoa(c.MaxLogAgeDays)) + if settingsPendingAutostart(startOnLogin, minimizeToTray, saved) { + autostartStatus.SetText("Pending: save settings to apply") + } else { + refreshAutostartStatus() + } + refreshTrayRestartHint(minimizeToTray.Checked != saved.KeepRunningInTray) + settingsStatus.SetText("") + updateSaveState() + } + cancelSettings := widget.NewButtonWithIcon("Cancel", theme.CancelIcon(), func() { + loadFields(saved) + }) + restoreDefaults := widget.NewButtonWithIcon("Defaults", theme.MediaReplayIcon(), func() { + loadFields(domain.DefaultConfig()) + }) + + return settingsFormFields{ + startOnLogin: startOnLogin, + autostartStatus: autostartStatus, + minimizeToTray: minimizeToTray, + trayRestartHint: trayRestartHint, + notifications: notifications, + themeSelect: themeSelect, + executionModeSelect: executionModeSelect, + overlapPolicySelect: overlapPolicySelect, + defaultTimeout: defaultTimeout, + configPath: paths.ConfigPath, + jobsFile: jobsFile, + jobsFileBrowse: jobsFileBrowse, + logsDir: logsDir, + logsDirOpen: logsDirOpen, + logsDirBrowse: logsDirBrowse, + maxLogFiles: maxLogFiles, + maxLogAgeDays: maxLogAgeDays, + saveSettings: saveSettings, + cancelSettings: cancelSettings, + restoreDefaults: restoreDefaults, + settingsStatus: settingsStatus, + } +}