Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 54 additions & 1 deletion SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -815,14 +815,67 @@ Four decisions, each taken over a plausible alternative:
the preset menus rather than growing its own copy of the ownership
handling those exist for.

### Phase 4s.1 - Config shared between instances ✅ done
Two qtxterm windows are two processes over one `cron.json` and one
`presets.json`. Each loaded its file once at launch, so a job added in one
was invisible to the other until restart - and the next save from the stale
window wrote its old list over the new one, losing the job silently.

- [x] **Polled once a minute, not watched.** The scheduler's tick already
runs every second, so checking on the minute costs a `stat()` and *no
extra wakeup*. A `QFileSystemWatcher` costs a worker thread, wakes on
every change in the config directory, and stops watching a file that
is *replaced* rather than modified - which is exactly what the atomic
save below does. A minute is also cron's own resolution: there is
nothing to do with a job noticed sooner than it could first fire.
- [x] **Stamped, so an unchanged file is never re-parsed.** `(mtime_ns,
size)`, compared before reading. Reloading unconditionally would emit
`changed` every minute and rebuild every menu, including one the user
has open.
- [x] **Presets are polled too, not just jobs.** A job added elsewhere
normally arrives with the Macro it names, and a job whose Macro has
not been re-read fails every single firing with "no Macro named ...
any more".
- [x] **Atomic save**, via a sibling temp file and `os.replace`. The reader
is another process polling this exact path; a plain write is visible in
its truncated middle.
- [x] **Reloads are suspended while an editor is open.** Both editors address
entries by index, and an `exec()` dialog still runs the event loop - a
poll landing mid-edit would shift the list under the open form and save
the user's changes onto whichever entry inherited that index. Counted,
not a flag, since two editors can be open over one store.
- [x] **A bad read changes nothing and is retried.** Invalid JSON - a
hand-edit in progress, a half-synced cloud drive, an older qtxterm
without the atomic save - leaves the in-memory list alone and does not
advance the stamp. Raising out of a timer would take down the window
over a file that is very likely fine a second later. A *deleted* file
is treated the same way: the presets do not evaporate mid-session.

Two things this deliberately does not fix:

- **The clobber window is narrowed, not closed.** Saves still write the whole
file, so an edit made in two windows inside the same minute still resolves
last-writer-wins. Closing it properly means merging per entry, which needs
stable ids the files do not have.
- **Both instances still fire the same job**, each into its own tab, since
each runs its own scheduler. Cron is per-instance by design. If that ever
needs to change, the fix is a single-instance lock at launch, not a
fourth thing for the poll to do.

Implementation note: `config_store.py` holds this once and both stores
inherit it - `PresetStore` and `CronStore` were already the same problem
twice (a list of dataclasses, one JSON file, editors addressing entries by
index), and the file handling is now the part they literally share.

Implementation notes:

- `cron.py` is schedules and storage and knows nothing about terminals;
`cron_scheduler.py` owns the tab-per-job and the firing. Splitting them is
what let the whole expression layer be tested without a Qt widget.
- The scheduler ticks every second and acts only when the *minute* changes.
A 60s timer drifts against the wall clock and skips a minute whenever the
machine sleeps.
machine sleeps. On each minute change it re-reads the config first, then
fires - see Phase 4s.1.
- `next_run()` walks minute by minute but skips whole days that cannot match,
and gives up after four years - "0 0 31 2 *" parses fine and can never
happen, and the alternative to a bound is a UI that hangs.
Expand Down
7 changes: 6 additions & 1 deletion src/qtxterm/assets/USAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -364,14 +364,19 @@ that menu, the same way Macros do - useful once you have a job per feed per
session. Ungrouped jobs stay at the top level. **Run Now** in the editor runs a job
immediately, which beats waiting until 2am to find out whether it works.

Two things to know:
Three things to know:

- **Jobs only run while qtxterm is open**, and nothing missed while it was
closed is caught up on. Launching after a weekend does not fire a burst of
overdue jobs. For work that must happen whether or not you're at the
machine, use the system's own scheduler.
- **A job names its Macro.** Rename or delete that Macro and the job says so
in the status bar rather than running something else.
- **Two qtxterm windows share one set of jobs.** Add or edit a job in one and
the other picks it up within a minute, no restart needed. Note that each
window runs its own schedule, so a job fires once per open window, each
into its own tab. If that isn't what you want, keep one window open, or
turn the job off in the others.

## Ordering

Expand Down
150 changes: 150 additions & 0 deletions src/qtxterm/config_store.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
"""Shared file handling for the JSON config stores.

`PresetStore` and `CronStore` are the same problem twice: a list of
dataclasses in one JSON file under the user's config dir, edited by dialogs
that address entries by index. This holds the parts that are identical - the
atomic write, and picking up a file a *second qtxterm instance* has written.

On the second instance: each process loads its config once at launch and
keeps it in memory, so without this a job added in one window is invisible to
the other, and the next save from the stale window writes its old list over
the new one. Polling closes most of that gap - see `reload_if_changed`.
"""

from __future__ import annotations

import json
import os
from contextlib import contextmanager
from pathlib import Path

from PySide6.QtCore import QObject, Signal

# What we compare to decide the file changed under us. mtime alone is too
# coarse on filesystems with a 1-2s timestamp granularity (FAT, some network
# shares), where two writes in the same second look identical; the size
# catches the common case of those two writes differing in length.
_Stamp = tuple[int, int] | None


class ConfigStore(QObject):
"""A JSON list on disk, reloaded when another process rewrites it.

Subclasses own the shape of the payload (`_apply_payload` /
`_build_payload`) and what an absent file means (`_apply_missing`); this
class owns the bytes.
"""

changed = Signal()

def __init__(self, path: Path) -> None:
super().__init__()
self.path = path
self._stamp: _Stamp = None
self._suspended = 0

# -- subclass hooks ----------------------------------------------------

def _apply_payload(self, raw: list) -> None:
raise NotImplementedError

def _build_payload(self) -> list:
raise NotImplementedError

def _apply_missing(self) -> None:
"""Called by load() when the file does not exist yet."""
raise NotImplementedError

# -- file handling -----------------------------------------------------

def load(self) -> None:
if not self.path.exists():
self._stamp = None
self._apply_missing()
return
self._apply_payload(json.loads(self.path.read_text(encoding="utf-8")))
self._stamp = self._current_stamp()

def save(self) -> None:
"""Write the whole file, atomically, and announce it.

Atomically because another instance may be polling this exact file:
a plain write is visible in its truncated middle, and the reader gets
a JSONDecodeError instead of a config. `os.replace` means a reader
sees either the old file or the new one.
"""
self.path.parent.mkdir(parents=True, exist_ok=True)
text = json.dumps(self._build_payload(), indent=2)
temp = self.path.with_name(f"{self.path.name}.{os.getpid()}.tmp")
try:
temp.write_text(text, encoding="utf-8")
os.replace(temp, self.path)
except OSError:
temp.unlink(missing_ok=True)
raise
self._stamp = self._current_stamp()
self.changed.emit()

def reload_if_changed(self) -> bool:
"""Re-read if another process has rewritten the file. Did we reload?

Cheap enough to call on a timer: a stat, and a parse only when the
stamp moved. Callers get `changed` exactly as if the edit had been
made here, so every menu and sidebar already knows what to do with it.

Never raises. A file that is unreadable *now* - mid-write by an older
qtxterm without the atomic save, half-synced by a cloud drive, or
hand-edited into invalid JSON - leaves the in-memory list alone and
is retried on the next poll. Throwing from a timer would take out the
window over a file that is very likely fine a second later.
"""
if self._suspended:
return False
stamp = self._current_stamp()
if stamp == self._stamp:
return False
if stamp is None:
# Deleted out from under us. Keeping what we have is the kinder
# reading: the file coming back is a reload, and until then the
# user's presets do not evaporate mid-session.
self._stamp = None
return False
try:
raw = json.loads(self.path.read_text(encoding="utf-8"))
except (OSError, ValueError):
return False
# Stamped only on success, so a failed read is retried rather than
# remembered as done.
self._apply_payload(raw)
self._stamp = stamp
self.changed.emit()
return True

def suspend_reload(self) -> None:
"""Hold off reloads while an editor is open over this store.

The editors address entries by index (`update(index, ...)`), and a
modal dialog still runs the event loop, so a poll landing mid-edit
would shift the list under the open form and save the user's changes
onto whichever entry inherited that index. Counted rather than a
flag, because more than one editor can be open over one store.
"""
self._suspended += 1

def resume_reload(self) -> None:
self._suspended = max(0, self._suspended - 1)

@contextmanager
def reload_suspended(self):
self.suspend_reload()
try:
yield
finally:
self.resume_reload()

def _current_stamp(self) -> _Stamp:
try:
info = self.path.stat()
except OSError:
return None
return (info.st_mtime_ns, info.st_size)
27 changes: 10 additions & 17 deletions src/qtxterm/cron.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@
from __future__ import annotations

import dataclasses
import json
from datetime import datetime, timedelta
from pathlib import Path

import platformdirs
from PySide6.QtCore import QObject, Signal

from qtxterm.config_store import ConfigStore

# (name, low, high) per field, in the order they are written.
_FIELDS = [
Expand Down Expand Up @@ -222,34 +222,27 @@ def default_cron_path() -> Path:
return Path(platformdirs.user_config_dir("qtxterm", appauthor=False)) / "cron.json"


class CronStore(QObject):
class CronStore(ConfigStore):
"""Loads/saves cron jobs as JSON, the same shape PresetStore uses."""

changed = Signal()

def __init__(self, path: Path | None = None) -> None:
super().__init__()
self.path = path or default_cron_path()
super().__init__(path or default_cron_path())
self.jobs: list[CronJob] = []
self.load()

def load(self) -> None:
if not self.path.exists():
self.jobs = []
return
raw = json.loads(self.path.read_text(encoding="utf-8"))
def _apply_missing(self) -> None:
self.jobs = []

def _apply_payload(self, raw: list) -> None:
# Unknown keys are dropped rather than raising: a file written by a
# later version should cost you a field, not the whole app.
fields = {f.name for f in dataclasses.fields(CronJob)}
self.jobs = [
CronJob(**{k: v for k, v in item.items() if k in fields}) for item in raw
]

def save(self) -> None:
self.path.parent.mkdir(parents=True, exist_ok=True)
payload = [dataclasses.asdict(job) for job in self.jobs]
self.path.write_text(json.dumps(payload, indent=2), encoding="utf-8")
self.changed.emit()
def _build_payload(self) -> list:
return [dataclasses.asdict(job) for job in self.jobs]

def add(self, job: CronJob) -> None:
self.jobs.append(job)
Expand Down
9 changes: 9 additions & 0 deletions src/qtxterm/cron_editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ def __init__(
self._preset_store = preset_store
self._scheduler = scheduler
self._current_index: int | None = None
# This form addresses jobs by index, so the lists behind it must not
# be swapped out by a poll while it is open - see ConfigStore.
self._store.suspend_reload()
self._preset_store.suspend_reload()
self.finished.connect(self._resume_reloads)

layout = QHBoxLayout(self)

Expand Down Expand Up @@ -129,6 +134,10 @@ def __init__(
if create_new:
self._new_job()

def _resume_reloads(self) -> None:
self._store.resume_reload()
self._preset_store.resume_reload()

def _runnable_presets(self):
"""Macros only.

Expand Down
22 changes: 22 additions & 0 deletions src/qtxterm/cron_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ class CronScheduler(QObject):
the app starts never fires, and nothing missed while it was closed is
replayed. Otherwise launching after a weekend would open a burst of
terminals before you had touched anything.

Config is re-read on the minute, so a job added in another instance is
picked up here without a restart - see `refresh_config`.
"""

job_fired = Signal(str)
Expand Down Expand Up @@ -73,8 +76,27 @@ def _on_tick(self) -> None:
if minute == self._last_minute:
return
self._last_minute = minute
self.refresh_config()
self.run_due_jobs(minute)

def refresh_config(self) -> None:
"""Pick up jobs and Macros written by another qtxterm instance.

Polled here rather than watched, for two reasons. This tick already
happens every second, so checking on the minute costs a stat and no
extra wakeup at all - where a QFileSystemWatcher costs a worker
thread, and silently stops watching when a file is *replaced*, which
is exactly what the atomic save in `ConfigStore` does. And a minute
is the resolution cron works at anyway; there is nothing to be done
with a job noticed sooner than the minute it could first fire.

Presets as well as jobs, because a job added elsewhere usually
arrives with the Macro it runs, and a job whose Macro we have not
re-read fails every firing with "no Macro named ... any more".
"""
self._preset_store.reload_if_changed()
self._cron_store.reload_if_changed()

def run_due_jobs(self, minute: datetime) -> list[str]:
"""Fire every enabled job whose schedule matches `minute`."""
fired: list[str] = []
Expand Down
4 changes: 4 additions & 0 deletions src/qtxterm/preset_editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ def __init__(
self.resize(620, 420)
self._store = store
self._current_index: int | None = None
# Addresses presets by index, so a poll must not reorder the list
# under the open form - see ConfigStore.
self._store.suspend_reload()
self.finished.connect(lambda _: self._store.resume_reload())
self._is_command = category == CATEGORY_COMMANDS
self._is_macro = category == CATEGORY_MACROS
self._is_selection = category == CATEGORY_SELECTION
Expand Down
Loading
Loading