aboutsummaryrefslogtreecommitdiff
path: root/cmd/krino/sort.go
diff options
context:
space:
mode:
Diffstat (limited to 'cmd/krino/sort.go')
-rw-r--r--cmd/krino/sort.go135
1 files changed, 66 insertions, 69 deletions
diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go
index c2038fd..12812c7 100644
--- a/cmd/krino/sort.go
+++ b/cmd/krino/sort.go
@@ -26,13 +26,12 @@ import (
// stdin is os.Stdin, threaded through this seam rather than referenced
// directly: cmdSort's terminal check, installSignalHandler and reviewDir
// all read it, and a test must never depend on what the ambient test
-// binary's stdin happens to be (fix round 2026-09-12/item 4). If it were
-// ever a real terminal, code that only worked by assuming otherwise would
-// fall through to the interactive prompt and block the test suite on a
-// keypress - the same kind of hang the lock-cancellation test was built to
-// never risk. Tests point this at something guaranteed non-terminal
-// (commands_test.go's home helper) instead of relying on a claim about
-// what go test does with stdin.
+// binary's stdin happens to be. If it were ever a real terminal, code that
+// only worked by assuming otherwise would fall through to the interactive
+// prompt and block the test suite on a keypress - the same kind of hang
+// the lock-cancellation test was built to never risk. Tests point this at
+// something guaranteed non-terminal (commands_test.go's home helper)
+// instead of relying on a claim about what go test does with stdin.
var stdin = os.Stdin
// stdinIsTerminal is the check that review can prompt at all, a seam so a
@@ -46,11 +45,11 @@ var stdinIsTerminal = func() bool { return term.IsTerminal(int(stdin.Fd())) }
// (and cannot drift) across the three places it applies.
const zeroOutcome = "0 applied · 0 failed · 0 declined"
-// cmdSort plans and, from Task 7, applies the included directories: flags
-// are checked before any config is read, a bad config stops the whole run
-// before scanning (spec §11), and one engine.Session - its log, run id and
-// claims - covers every directory in the run. See docs/design.md
-// §8.2-§8.4 and §11 for the flow this follows.
+// cmdSort plans and applies the included directories: flags are checked
+// before any config is read, a bad config stops the whole run before
+// scanning (spec §11), and one engine.Session - its log, run id and claims
+// - covers every directory in the run. See docs/design.md §8.2-§8.4 and
+// §11 for the flow this follows.
func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
if g.yes && g.dry {
return usageError(stderr, "-y and -n cannot be used together")
@@ -80,9 +79,9 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
return usageError(stderr, "refusing to prompt: stdin is not a terminal (use -y or -n)")
}
- // Ruling 5: internal/tui deliberately does not trap signals - a package
- // that installs process-wide handlers as a side effect of reading one
- // key would surprise every caller. It lands here because cmdSort must
+ // internal/tui deliberately does not trap signals - a package that
+ // installs process-wide handlers as a side effect of reading one key
+ // would surprise every caller. It lands here because cmdSort must
// install one anyway: spec §11 says Ctrl-C finishes the current step,
// logs it, and stops, which means the ctx passed to Apply below must be
// cancelled on SIGINT. The same handler also restores the terminal on
@@ -96,8 +95,8 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
// The run itself - the log, its run id and the claims every directory
// shares - belongs to the engine, so the GUI runs a directory exactly
- // as this does (GUI design §1.3). Ruling 2: a dry session opens no log,
- // since journal.Open creates the state directory and an empty krino.log
+ // as this does (GUI design §1.3). A dry session opens no log, since
+ // journal.Open creates the state directory and an empty krino.log
// merely by being called.
sess, err := e.NewSession(g.dry)
if err != nil {
@@ -125,9 +124,8 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
// Spec §3/§11: a second krino on the same directory waits for the
// lock, or fails immediately with -y, so a cron job never piles up
// behind a stuck run. lock.Acquire takes ctx precisely so that wait
- // is not unbounded in practice (fix round 2026-09-12/item 1): a
- // signal cancels it and Acquire returns ctx.Err() promptly instead
- // of polling forever.
+ // is not unbounded in practice: a signal cancels it and Acquire
+ // returns ctx.Err() promptly instead of polling forever.
l, err := sess.Lock(ctx, d, !g.yes)
if err != nil {
if interrupted(err) {
@@ -176,15 +174,15 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
}
if g.json {
// --json is only ever reached with -n (checked above), and
- // Ruling 7 is explicit that JSON must never be paged, so
- // this returns before any of the paging/review code below.
+ // JSON must never be paged, so this returns before any of
+ // the paging/review code below.
jsonDirs = append(jsonDirs, plan.NewJSONDir(d.Name, d.Root, dp.Chains, fileNotes(dp.Result), dp.Result.Warnings))
return false
}
- // Ruling 6: the plan goes through tui.Page - taller than the
- // terminal, it is shown through $PAGER and the prompt follows
- // once the pager exits (spec §8.2) - for -n as much as for the
+ // The plan goes through tui.Page - taller than the terminal, it
+ // is shown through $PAGER and the prompt follows once the
+ // pager exits (spec §8.2) - for -n as much as for the
// interactive and -y paths; a dry run that scrolls 200 files
// off the top of the terminal is exactly the case the pager
// exists for. printPlan is reused as-is (render.go), never
@@ -228,8 +226,8 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
case 's':
// Spec §8.2: [s] applies nothing in this directory and
// moves on - no Apply call at all, so nothing is logged
- // for it either (Ruling 1: this is a chosen outcome, not a
- // failure, and must not set exit 1).
+ // for it either: this is a chosen outcome, not a failure,
+ // and must not set exit 1.
fmt.Fprintln(stdout, zeroOutcome)
return false
case 'q':
@@ -254,11 +252,11 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
}
res, aerr := sess.Apply(ctx, toApply, approved)
if aerr != nil {
- // Interrupted mid-apply (fix round 2026-09-12/item 2):
- // treated exactly like the cancelled lock wait above - not
- // a failure ("context canceled" is a Go-ism, not something
- // to show a user who just pressed Ctrl-C). The ctx.Err()
- // check at the end of this function turns it into exit 130.
+ // Interrupted mid-apply: treated exactly like the
+ // cancelled lock wait above - not a failure ("context
+ // canceled" is a Go-ism, not something to show a user who
+ // just pressed Ctrl-C). The ctx.Err() check at the end of
+ // this function turns it into exit 130.
if !interrupted(aerr) {
fmt.Fprintf(stderr, "krino: %s: %v\n", d.Name, aerr)
exit = 1
@@ -266,8 +264,8 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
return stopAfterApply(action, aerr)
}
fmt.Fprintln(stdout, withNotReviewed(outcome(p, res.Applied, res.Failed, res.Declined), notReviewed))
- // Ruling 1: only an actual step failure makes the run exit 1
- // here - a directory the user declined or skipped must not.
+ // Only an actual step failure makes the run exit 1 here - a
+ // directory the user declined or skipped must not.
if res.Failed > 0 {
exit = 1
}
@@ -305,12 +303,12 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
}
// actionableChains returns the chains of dp.Chains that have at least one
-// step that will actually run (chainActing, render.go) - fix wave item 4 /
-// Minor 5: this used to be a separate len(c.Steps) > 0 check, which
-// disagreed with render.go's countActing over a chain every one of whose
-// steps is skipped, so a directory could report "0 to act on" and then
-// still offer such a chain for approval. Converged on chainActing, this is
-// now also stricter than the filter engine.Apply's own forward-path loop
+// step that will actually run (chainActing, render.go). This used to be a
+// separate len(c.Steps) > 0 check, which disagreed with render.go's
+// countActing over a chain every one of whose steps is skipped, so a
+// directory could report "0 to act on" and then still offer such a chain
+// for approval. Converged on chainActing, this is now also stricter than
+// the filter engine.Apply's own forward-path loop
// applies (internal/engine/apply.go's Apply, still len(c.Steps) > 0): an
// all-skipped chain is simply never a candidate for approval here, so it
// can never reach Apply with approved == true, and Apply's own loop -
@@ -328,12 +326,12 @@ func actionableChains(chains []plan.Chain) []plan.Chain {
// installSignalHandler arranges for SIGINT and SIGTERM to cancel cancel
// and, if stdin is a terminal, restore it to the state it was in when this
-// was called (Ruling 5). The returned func stops the handler and must be
-// called once the run is over, or its goroutine and signal registration
-// outlive cmdSort.
+// was called. The returned func stops the handler and must be called once
+// the run is over, or its goroutine and signal registration outlive
+// cmdSort.
//
-// Fix round 2026-09-12/item 2: the handler loops rather than servicing one
-// signal and exiting. A single-shot select left signal.Notify's
+// The handler loops rather than servicing one signal and exiting. A
+// single-shot select left signal.Notify's
// registration in place (which suppresses Go's default terminate) with no
// goroutine left reading the channel, so a second Ctrl-C landed in the
// buffered channel unread and a third was dropped outright - together with
@@ -356,8 +354,8 @@ func installSignalHandler(cancel context.CancelFunc) func() {
sig := make(chan os.Signal, 1)
signal.Notify(sig, os.Interrupt, syscall.SIGTERM)
// A hangup stops krino like SIGTERM - unless it was started ignoring
- // hangups (nohup), which must go on meaning "keep running" (re-review
- // pa F3): asking for SIGHUP would re-enable it.
+ // hangups (nohup), which must go on meaning "keep running": asking for
+ // SIGHUP would re-enable it.
if !signal.Ignored(syscall.SIGHUP) {
signal.Notify(sig, syscall.SIGHUP)
}
@@ -374,20 +372,19 @@ func installSignalHandler(cancel context.CancelFunc) func() {
signals++
if signals >= 2 {
// Deliberate exception to "the lock is released on
- // every path" (fix round 2026-09-12/item 3, by
- // design, documented on review): every deferred
- // cleanup in cmdSort - including the held directory's
- // lock.Release - is skipped here. That is intentional:
- // this is the user's escape hatch when a clean
- // shutdown was already asked for once (the first
- // signal) and not delivered, so trying to unwind
- // cleanly a second time is exactly what would make the
- // hatch unreliable. It is safe to skip that unwind:
- // journal.Append flushes each line as it writes, so
- // nothing buffered is lost by exiting immediately, and
- // an abandoned lock file is reclaimed automatically by
- // Task 4's stale-pid takeover the next time anything
- // tries to acquire it (lock.go's tryAcquire).
+ // every path": every deferred cleanup in cmdSort -
+ // including the held directory's lock.Release - is
+ // skipped here. That is intentional: this is the
+ // user's escape hatch when a clean shutdown was
+ // already asked for once (the first signal) and not
+ // delivered, so trying to unwind cleanly a second time
+ // is exactly what would make the hatch unreliable. It
+ // is safe to skip that unwind: journal.Append flushes
+ // each line as it writes, so nothing buffered is lost
+ // by exiting immediately, and an abandoned lock file is
+ // reclaimed automatically by the stale-pid takeover
+ // the next time anything tries to acquire it (lock.go's
+ // tryAcquire).
os.Exit(130)
}
case <-done:
@@ -497,17 +494,17 @@ func printSkipped(w io.Writer, skipped []scan.Skipped) {
}
}
-// skipReasonOrder is plan 2's reviewed order for the skip reasons the last
-// line reports, before "unmatched".
+// skipReasonOrder is the order the skip reasons the last line reports,
+// before "unmatched".
var skipReasonOrder = []scan.Reason{scan.Ignored, scan.Busy, scan.TooNew, scan.TooBig, scan.Symlink, scan.NotRegular, scan.Unreadable}
// skipSummaryLine builds the "not acted on: N ignored · N busy · ... · N
// unmatched" line per spec §8.2's item format ("<count> <label>", not
// "<label>: <count>"), only the non-zero counts, or "" when every count is
-// zero. Ordering is plan 2's reviewed skipReasonOrder, with "unmatched"
-// last: the spec's own worked example shows only three of the seven
-// categories and states no ordering rule, so its incidental order is not
-// adopted, only its item format and unmatched's trailing position.
+// zero. Ordering follows skipReasonOrder, with "unmatched" last: the
+// spec's own worked example shows only three of the seven categories and
+// states no ordering rule, so its incidental order is not adopted, only
+// its item format and unmatched's trailing position.
func skipSummaryLine(r *engine.Result, chains []plan.Chain, verbose bool) string {
counts := map[scan.Reason]int{}
for _, s := range r.Skipped {
@@ -600,8 +597,8 @@ func interrupted(err error) bool {
}
// stopAfterApply reports whether krino stops once a directory has been
-// applied: an interrupt stops it, and so does [w], whether or not its apply
-// succeeded (review cli F3) - no later directory is planned or asked about.
+// applied: an interrupt stops it, and so does [w], whether or not its
+// apply succeeded - no later directory is planned or asked about.
func stopAfterApply(action rune, err error) bool {
return interrupted(err) || action == 'w'
}
generated by cgit v1.3 (git 2.53.0) at 2026-09-19 09:14:08 +0000