Artifact 7a7af9578851cdf2021dd98da60c22a897cd5c12f491395bb6d8abf3e7ed4b09:
- File
internal/deck/deck.go
— part of check-in
[527759acd5]
at
2026-07-19 18:31:23
on branch trunk
— fix: distinguish absent, empty, and valued U-cards in manifest parsing (#73)
* fix: distinguish absent, empty, and valued U-cards in manifest parsing
Manifest parsing collapsed a wholly absent U-card and a present-but-empty U-card into the same stored value (empty string), so every crosslink call site that wrote the check-in user into event/attachment/forumpost rows lost the distinction between "no login recorded" (should be SQL NULL) and "login explicitly blanked" (canonical fossil resolves this to the literal "anonymous" at parse time — see src/manifest.c:1008-1016).
Change deck.Deck.U from string to *string so the three states are representable: nil (absent), non-nil "anonymous" (present-empty, resolved in parseUCard the same way fossil's own parser does), and non-nil with a value (present). Resolving once in the parser means every downstream crosslink/rebuild call site can keep binding d.U directly — Go's database/sql converts a nil *string argument to SQL NULL and dereferences a non-nil one automatically, so no per-call-site special casing is needed. The type change itself makes the old ambiguous string-binding path a compile error everywhere Deck.U is constructed, closing off the drift that caused issue #29.
Added deck.Str, a small pointer-literal helper, since Go can't take the address of a string literal directly; existing tests that built Deck{U: "..."} now go through it.
* fix: review nits — whitespace U-cards, rebuild coverage, tag.AddTag, naming
Four follow-ups from review of the U-card presence fix:
- deck.parseUCard now trims args before the emptiness check, matching canonical fossil's next_token() (skips leading whitespace) and every sibling card parser in this file. Previously "U \n" stored literal whitespace instead of resolving to "anonymous". Added a whitespace-only case to TestParseUCardStates.
- Added TestRebuild_UCardPresenceStates covering the rebuild path (rebuild_manifest.go's rebuildCheckin), mirroring TestCrosslink_UCardPresenceStates. The binding site was already correct (same nil-to-NULL pointer semantics as crosslink.go) but had no dedicated regression test.
- tag.AddTag now panics on an empty opts.User, matching the panic idiom CheckinOpts.User already establishes. Previously a tag created with an empty User silently crosslinked to user=''; that asymmetry with CheckinOpts was a wart worth closing rather than documenting as intentional.
- Renamed deck.Str to deck.User: the name should say what the helper constructs (a Deck.U value), not how it's implemented (a pointer take). (user: [email protected], size: 2178) [annotate] [blame] [check-ins using]
package deck import "time" type ArtifactType int const ( Checkin ArtifactType = iota Wiki Ticket Event Cluster ForumPost Attachment Control ) type Deck struct { Type ArtifactType A *AttachmentCard B string C string D time.Time E *EventCard F []FileCard G string H string I string J []TicketField K string L string M []string N string P []string Q []CherryPick R string T []TagCard // U tracks the check-in user with three distinct states, mirroring // fossil's src/manifest.c:1008-1016 U-card handling: // - nil: no U-card in the manifest at all (SQL NULL at crosslink time) // - non-nil, "anonymous": U-card present but empty (resolved at parse // time, matching canonical fossil's own substitution) // - non-nil, non-empty: U-card present with a login name // Deliberately *string rather than string: a bare "" cannot represent // "absent" and "present-but-empty" at once, which previously collapsed // both into the same stored value. See User for constructing literals. U *string W []byte Z string } type FileCard struct { Name string UUID string Perm string OldName string } type TagCard struct { Type TagType Name string UUID string Value string } type TagType byte const ( TagSingleton TagType = '+' TagPropagating TagType = '*' TagCancel TagType = '-' ) type CherryPick struct { IsBackout bool Target string Baseline string } type AttachmentCard struct { Filename string Target string Source string } type EventCard struct { Date time.Time UUID string } type TicketField struct { Name string Value string } // User returns a pointer to s for constructing Deck.U. Go forbids taking // the address of a string literal directly, so composite literals go // through this instead of Deck{U: &s}. Deliberately *string rather than a // named struct type: passing *string straight to database/sql lets a nil // argument bind SQL NULL and a non-nil one deref to its value automatically // (see Deck.U), so crosslink/rebuild call sites need no special-casing. func User(s string) *string { return &s }