Skip to content

feature/linux tray utility - #3

Merged
Recluse merged 2 commits into
Recluse:mainfrom
nett00n:feature/linux-tray-utility
Sep 23, 2026
Merged

Recluse merged 2 commits into
Recluse:mainfrom
nett00n:feature/linux-tray-utility

Conversation

@nett00n

@nett00n nett00n commented Sep 23, 2026

Copy link
Copy Markdown
Contributor
  • read only system tray for linux
  • full interactivity for the linux tray

@nett00n nett00n closed this Sep 23, 2026
@nett00n nett00n reopened this Sep 23, 2026
@Recluse

Recluse commented Sep 23, 2026

Copy link
Copy Markdown
Owner

Thanks for this — it turned out to be a larger and more careful piece of work than I expected, and the split is exactly the one I would have asked for: jobs/mod.rs asks the question, the individual backends provide the answers, and the freshness logic passes through the whole thing without being disturbed.

A few details stood out while I was reading the diff. TriggerState is a genuine improvement over what macOS can provide, and you went beyond simply adding another field by carrying the idea through to the point where it actually matters: since_last_output prefers the trigger over the log, which means that a job that ran once and then stalled can still be detected on Linux even if it never writes a log at all. An invalid LastTriggerUSec also becomes a fault instead of quietly turning into “never ran”, while run_now waits for the InvocationID to change before attributing an exit code to the run it just requested, which is the same reasoning we ended up using on the launchd side, independently. You also caught the .timer suffix eating the job name in short(), which is exactly the sort of thing I would otherwise have discovered the hard way.

Putting the tray behind an off-by-default feature is also the right choice, as is splitting CI in a way that leaves the two preceding steps as independent proof that the rest of the project does not need GTK.

There is one blocker in console/src/jobs/launchd.rs:528: inside mod tests, super::TriggerState resolves to launchd rather than jobs; the same form works on lines 79 and 104 only because those references are at module level. As a result, cargo test does not compile on macOS at all, leaving the entire console test suite dead there while Linux CI remains green simply because launchd.rs is excluded by cfg. Removing super:: fixes it, since TriggerState is already in scope through use super::*.

There are also two smaller issues. In systemd.rs:108, show(&service_label).unwrap_or_default() means that if the .service cannot be read, the job still renders but loses its exit code, PID and log path without giving any indication of what went wrong; the equivalent timer failure becomes an explicit fault only a few lines above, so swallowing the service error here feels out of place in a file that is otherwise careful not to hide this kind of failure. There is also a duplicated sentence in the doc comment at view.rs:15, and launchd.rs:10 imports SystemTime without using it.

As a bit of housekeeping, the branch is currently three commits behind main, and CI has not run on the PR at all. I merged it onto the current main locally and the merge is clean, with nothing in your changes conflicting, but it is still worth rebasing so that CI exercises the code that will actually land rather than an older base.

On my side, I built the project and ran the test suite on macOS, which is how the compilation break above surfaced, read through the entire diff, and merged the branch onto current main to check what happens to the workflow file. I have not tested any of the Linux behaviour, though: I do not have a Linux machine available here, so the systemd half of this review is based entirely on reading the code, and someone should run it against real timers before this lands.

@Recluse
Recluse merged commit 455fc9a into Recluse:main Sep 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants