aboutsummaryrefslogtreecommitdiff
path: root/internal/journal/read.go
diff options
context:
space:
mode:
authorLukasz Kasprzak <lukas@labunix.xyz>2026-09-17 12:11:42 +0200
committerLukasz Kasprzak <lukas@labunix.xyz>2026-09-17 12:11:42 +0200
commitbddbd74e4a73e8e32bcf648efd1cac5655f6d0cd (patch)
tree187e6a1fb722d9ab97d2076f69f997f5d371e943 /internal/journal/read.go
parentcd7425b81f963a948f0abe7df3f9e58e190c2b78 (diff)
downloadkrino-bddbd74e4a73e8e32bcf648efd1cac5655f6d0cd.tar.gz
krino-bddbd74e4a73e8e32bcf648efd1cac5655f6d0cd.zip
comments that explain the code, not how it was written
About 340 comments cited the development process: task and plan numbers, fix waves, rulings, reviewers, and the author in the third person with a date. None of that exists outside the work itself, so to a reader it pointed at nothing. Each one now states the engineering reason it was standing in front of; where a comment was provenance and nothing else, it is gone. References to docs/design.md and docs/gui-design.md by section stay: both ship with the repository. The design documents lose their amendment diaries - CHANGELOG.md is that record - and the GUI's says plainly that the window has gone further than the document. Only comments changed. Every .go file was parsed and its code printed with comments stripped, before and after: the two hashes are identical across all 175 files.
Diffstat (limited to 'internal/journal/read.go')
-rw-r--r--internal/journal/read.go110
1 files changed, 55 insertions, 55 deletions
diff --git a/internal/journal/read.go b/internal/journal/read.go
index 5a1de04..aa79118 100644
--- a/internal/journal/read.go
+++ b/internal/journal/read.go
@@ -25,12 +25,12 @@ const wantFields = 13
const undoOfPrefix = "undo of "
// UndoOf returns the Detail value an undo run's run-start entry carries to
-// record which run it reverses (see undoOfPrefix). Fix wave item 2 /
-// final-wave item 17: before this, internal/engine wrote the same text as
-// a bare string literal with nothing tying it to undoOfPrefix, so a typo in
-// either would silently break Runs' Undone marking while every test stayed
-// green. This is the one place that string is built; internal/engine calls
-// it rather than keeping its own copy.
+// record which run it reverses (see undoOfPrefix). Before this,
+// internal/engine wrote the same text as a bare string literal with
+// nothing tying it to undoOfPrefix, so a typo in either would silently
+// break Runs' Undone marking while every test stayed green. This is the
+// one place that string is built; internal/engine calls it rather than
+// keeping its own copy.
func UndoOf(run string) string {
return undoOfPrefix + run
}
@@ -44,8 +44,8 @@ type Run struct {
Counts map[string]int // action -> count of status "ok"
Undone bool // a later run reversed this one
// PartlyUndone is set with Undone while fewer of the run's reversible
- // steps have been reversed, over all its undo runs, than it took
- // (triage 34l): some were declined, refused or failed.
+ // steps have been reversed, over all its undo runs, than it took: some
+ // were declined, refused or failed.
PartlyUndone bool
UndoOf string // for an undo run, the run it reverses; "" otherwise
}
@@ -67,10 +67,10 @@ type ReversedKey struct {
// ReversedSteps counts, for runID, every reversal that earlier undo runs of
// it completed ("ok" undo- entries of runs whose run-start says they undo
-// runID), so a later undo of the same run can offer only what is left
-// (review M10). An undo run's own unparsable lines are skipped; a missing
-// reversal is then offered again, where its own checks refuse it if it had
-// in fact happened.
+// runID), so a later undo of the same run can offer only what is left. An
+// undo run's own unparsable lines are skipped; a missing reversal is then
+// offered again, where its own checks refuse it if it had in fact
+// happened.
func ReversedSteps(path, runID string) (map[ReversedKey]int, error) {
lines, err := readLines(path)
if err != nil {
@@ -93,19 +93,20 @@ func ReversedSteps(path, runID string) (map[ReversedKey]int, error) {
return out, nil
}
-// Entries returns every entry belonging to runID, in file order. Since plan
-// 10 (re-review N1), an unparsable line whose run column names another run
-// is ignored, and one of this run whose directory and file columns are still
-// readable is returned as a "damaged" entry for that file, so undo refuses
-// that file alone. Otherwise a line that fails to parse is skipped, but
-// Entries fails closed within the run's own window - from its run-start line to its run-end line, or to end of
-// file when there is no run-end (a crashed run, which is precisely when
-// corruption is likely): any unparsable line found inside that window sets
-// the returned error, whether or not the line's own Run column can still be
-// read back. The mere possibility that it belonged to this run is enough,
-// because an incomplete chain must refuse the whole run rather than let an
-// undo reverse it partway (spec §10). A line outside the window is ignored
-// even when unparsable, since it cannot belong to this run.
+// Entries returns every entry belonging to runID, in file order. An
+// unparsable line whose run column names another run is ignored, and one
+// of this run whose directory and file columns are still readable is
+// returned as a "damaged" entry for that file, so undo refuses that file
+// alone. Otherwise a line that fails to parse is skipped, but Entries
+// fails closed within the run's own window - from its run-start line to
+// its run-end line, or to end of file when there is no run-end (a crashed
+// run, which is precisely when corruption is likely): any unparsable line
+// found inside that window sets the returned error, whether or not the
+// line's own Run column can still be read back. The mere possibility that
+// it belonged to this run is enough, because an incomplete chain must
+// refuse the whole run rather than let an undo reverse it partway (spec
+// §10). A line outside the window is ignored even when unparsable, since
+// it cannot belong to this run.
//
// The residual risk this leaves is a false refusal, not a false success:
// krino's lock is per directory, not global, so two processes could in
@@ -154,7 +155,7 @@ func Entries(path, runID string) ([]Entry, error) {
run, runFound := runFieldOf(line)
if runFound && run != runID {
// Another run's damaged line: runs of different directories can
- // interleave, and it says nothing about this one (re-review N1).
+ // interleave, and it says nothing about this one.
continue
}
ours := inWindow || (runFound && run == runID)
@@ -165,7 +166,7 @@ func Entries(path, runID string) ([]Entry, error) {
// A line of this run cut or damaged where its file is still
// readable: that file's chain may be missing a step, so it is
// returned as damaged and PlanUndo refuses just that file; the
- // rest of the run stays undoable (re-review N1).
+ // rest of the run stays undoable.
out = append(out, Entry{Run: runID, Dir: dir, File: file, Action: "damaged", Status: "damaged", Detail: fmt.Sprintf("line %d", i+1)})
continue
}
@@ -185,8 +186,8 @@ func Entries(path, runID string) ([]Entry, error) {
// runFieldOf best-effort extracts a line's Run column even when the line
// otherwise fails to parse, so Entries can tell whether an unparsable line
// belonged to the run it was asked for. The column counts only when a tab
-// ends it: a line cut inside it holds a prefix of some run's ID, which names
-// no run (plan 10 re-check R2).
+// ends it: a line cut inside it holds a prefix of some run's ID, which
+// names no run.
func runFieldOf(line string) (string, bool) {
f := strings.SplitN(line, "\t", 3)
if len(f) < 3 {
@@ -214,13 +215,13 @@ func fileFieldsOf(line string) (dir, file string, ok bool) {
//
// A run is marked Undone when a later run's run-start entry's Detail is
// undoOfPrefix followed by this run's ID, AND that later run actually
-// reversed something (fix wave item 2): a fully declined undo - every file
-// the reviewer chose not to reverse - still opens with that same run-start
-// (ApplyUndo logs a declined file exactly as spec §9 asks the forward path
-// to), so the Detail alone is not proof anything happened. Reproduced by
-// the reviewer: `krino undo` with every file declined left `krino log`
-// reporting the original run "(undone)" regardless. What actually happened
-// is provable from the same file: at least one "ok" undo-* entry.
+// reversed something: a fully declined undo - every file declined rather
+// than reversed - still opens with that same run-start (ApplyUndo logs a
+// declined file exactly as spec §9 asks the forward path to), so the
+// Detail alone is not proof anything happened. Without this check, `krino
+// undo` with every file declined would leave `krino log` reporting the
+// original run "(undone)" regardless. What actually happened is provable
+// from the same file: at least one "ok" undo-* entry.
func Runs(path string, n int) ([]Run, error) {
lines, err := readLines(path)
if err != nil {
@@ -230,9 +231,8 @@ func Runs(path string, n int) ([]Run, error) {
order := make([]string, 0)
byID := make(map[string]*Run)
pendingUndo := make(map[string]string) // undo run ID -> the run ID it claims to undo
- // A file whose chain ended in a permanent delete is never undone, so its
- // reversible steps do not count toward what a run took (plan 11 review
- // L8).
+ // A file whose chain ended in a permanent delete is never undone, so
+ // its reversible steps do not count toward what a run took.
type fileOf struct{ run, dir, file string }
reversibleOf := map[fileOf]int{}
deletedFile := map[fileOf]bool{}
@@ -312,22 +312,22 @@ func Runs(path string, n int) ([]Run, error) {
return runs, nil
}
-// ranAnyUndoStep reports whether counts - a run's own tally of "ok" actions,
-// by action name - includes at least one undo- action that actually
-// restored something, as opposed to merely having been started and then
-// declining every file (Important 2), or having failed to restore anything
-// while a wholly unrelated undo-mkdir still happened to succeed (the
-// coordinator's tightening of that same fix): "undo-mkdir" is deliberately
-// excluded, the one undo- action package journal cannot help but name
-// directly (this package must not import internal/engine to reuse its
-// isFileAffecting predicate - journal is the lower layer), but which draws
-// exactly the same line that predicate does. Removing a directory once it
-// turns out empty is tidiness, not a restoration: a file's own chain stops
-// after a failed file-affecting reversal, but a failed or refused
-// undo-mkdir never stops anything (see internal/engine's isFileAffecting
-// and undoFile), so it can succeed for one file while every file-affecting
-// reversal in the whole run failed - and marking the original run Undone
-// from that alone would be Important 2's bug again, by a narrower route.
+// ranAnyUndoStep reports whether counts - a run's own tally of "ok"
+// actions, by action name - includes at least one undo- action that
+// actually restored something, as opposed to merely having been started
+// and then declining every file, or having failed to restore anything
+// while a wholly unrelated undo-mkdir still happened to succeed:
+// "undo-mkdir" is deliberately excluded, the one undo- action package
+// journal cannot help but name directly (this package must not import
+// internal/engine to reuse its isFileAffecting predicate - journal is the
+// lower layer), but which draws exactly the same line that predicate
+// does. Removing a directory once it turns out empty is tidiness, not a
+// restoration: a file's own chain stops after a failed file-affecting
+// reversal, but a failed or refused undo-mkdir never stops anything (see
+// internal/engine's isFileAffecting and undoFile), so it can succeed for
+// one file while every file-affecting reversal in the whole run failed -
+// and marking the original run Undone from that alone would be the same
+// bug again, by a narrower route.
func ranAnyUndoStep(counts map[string]int) bool {
for action, n := range counts {
if n > 0 && action != "undo-mkdir" && strings.HasPrefix(action, "undo-") {