Give monitorctl and monitord a real front door - #50
Merged
Conversation
`monitord --help` started the daemon. So did `-h`, `help`, `--version` and `--nonsense`: every one fell through to the start path, wrote a CSV and printed nothing. The usage text was compiled into the binary and complete, with no route to a terminal. `monitorctl` rejected an unknown command but not an unknown flag, so `--intrval 0.1` sampled at the default rate and said nothing. Both are now ParsableCommands, so the help is rendered from the @option and @Flag declarations and an unrecognised flag is refused by the same table. A flag added here appears in --help because there is no second place to add it to. - `--help`, `-h` and `help` print usage to stdout and exit 0. - `--version` prints the version and the commit it was built from. - An unknown flag, a missing value or an unparseable value is named on stderr and exits 64. Nothing starts. - A value that parses but cannot work is rejected in validate(): a non-positive --interval, a --count below one. - Choices come from the type — LogRetention.allValueStrings and the source registry's allIDs — not from a list written out in prose. SourceRegistry.allIDs becomes a stored property. It called makeAll() on every access, and makeAll() builds real readers: SMCSource opens an IOKit connection. That was cheap while the app asked once at launch and not cheap at all once the list went into a --help string ArgumentParser rebuilds on every parse — the new suite took four minutes before this and takes under a second after it. This adds the package's first third-party dependency, Apple's swift-argument-parser, used only by the two CLIs. Package.resolved is now committed rather than ignored: a version range resolves to whatever is newest on the day, and a release built from a different revision than the one tested is not the release that was tested. AGENTS.md and docs/ui.md drop the no-dependencies rule. CommandLineTests covers both binaries' front doors, which no other suite touches — the daemons themselves were never broken. CI additionally runs --help and --version against the real release binaries and asserts that `monitord --help` leaves its log directory empty, since a daemon booting instead of printing is only visible from outside the process.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ParsableCommands (swift-argument-parser), so--helpis rendered from the@Option/@Flagdeclarations and an unrecognised flag is refused by the same table. A flag added here appears in--helpbecause there is no second place to add it to.Sources/monitord/Monitord.swift,Sources/monitorctl/Monitorctl.swift.--help/-h/helpprint usage to stdout and exit 0.--versionprints the version and the commit it was built from. An unknown flag, a missing value or an unparseable value is named on stderr and exits 64, and nothing starts. A value that parses but cannot work — a non-positive--interval, a--countbelow one — is rejected invalidate().LogRetention.allValueStringsand the source registry'sallIDs, rather than a list written out in prose that was already one literal away from lying.SourceRegistry.allIDsbecomes a stored property (Sources/MonitorSources/SourceRegistry.swift:27). It calledmakeAll()on every access, andmakeAll()builds real readers —SMCSourceopens an IOKit connection. Cheap while the app asked once at launch, and not cheap at all once the list went into a--helpstring ArgumentParser rebuilds on every parse: the new suite took four minutes before this and takes under a second after it.swift-argument-parser, used only by the two CLI targets — nothing in the app, the UI or the sources links it.Package.resolvedis now committed rather than ignored, because a version range resolves to whatever is newest on the day and a release built from a different revision than the one tested is not the release that was tested.AGENTS.mdanddocs/ui.mddrop the no-dependencies rule.Closes #48
Test plan
swift test— 257 tests in 32 suites pass.CommandLineTestsis new and covers both binaries' front doors, which no other suite touches: the daemons themselves were never broken.swift build -c releaseswiftformat Sources Tests Plugins --lint --cache ignore— cleanThe issue's table re-run against the release binaries. All five argv rows now print and exit instead of booting a daemon, and none of them writes a CSV:
--help-hhelp--version1.6.0 (v1.6.0-…)--nonsenseCI green, including two new steps:
--help/--versionagainst the real release binaries, and an assertion thatmonitord --helpleaves its--dirempty — a daemon booting instead of printing is only visible from outside the process.Out of scope
The per-fan
sensor.fan.N.modecolumn asked for in the second comment on #48 is a metric change rather than a parsing one, and is not in this PR.