Artifact cf624453ddb69aef8eada2a5038549a96e1f4dae7835b5e1b964e05ab68b567c:
- File
internal/deck/marshal.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: 3385) [annotate] [blame] [check-ins using]
package deck import ( "fmt" "sort" "strings" ) func (d *Deck) Marshal() ([]byte, error) { if d == nil { panic("deck.Marshal: d must not be nil") } var b strings.Builder marshalCards(&b, d) body := b.String() zHash := computeZ([]byte(body)) return []byte(fmt.Sprintf("%sZ %s\n", body, zHash)), nil } // marshalCards writes all cards in ASCII order to the builder. func marshalCards(b *strings.Builder, d *Deck) { if b == nil { panic("deck.marshalCards: b must not be nil") } if d == nil { panic("deck.marshalCards: d must not be nil") } // Cards in strict ASCII order: A B C D E F G H I J K L M N P Q R T U W Z if d.A != nil { b.WriteString("A ") b.WriteString(FossilEncode(d.A.Filename)) b.WriteString(" ") b.WriteString(d.A.Target) if d.A.Source != "" { b.WriteString(" ") b.WriteString(d.A.Source) } b.WriteString("\n") } if d.B != "" { fmt.Fprintf(b, "B %s\n", d.B) } if d.C != "" { fmt.Fprintf(b, "C %s\n", FossilEncode(d.C)) } if !d.D.IsZero() { fmt.Fprintf(b, "D %s\n", d.D.UTC().Format("2006-01-02T15:04:05.000")) } if d.E != nil { fmt.Fprintf(b, "E %s %s\n", d.E.Date.UTC().Format("2006-01-02T15:04:05"), d.E.UUID) } if len(d.F) > 0 { sorted := make([]FileCard, len(d.F)) copy(sorted, d.F) sort.Slice(sorted, func(i, j int) bool { return sorted[i].Name < sorted[j].Name }) for _, f := range sorted { b.WriteString("F ") b.WriteString(FossilEncode(f.Name)) if f.UUID != "" { b.WriteString(" ") b.WriteString(f.UUID) if f.Perm != "" { b.WriteString(" ") b.WriteString(f.Perm) } } if f.OldName != "" { b.WriteString(" ") b.WriteString(FossilEncode(f.OldName)) } b.WriteString("\n") } } if d.G != "" { fmt.Fprintf(b, "G %s\n", d.G) } if d.H != "" { fmt.Fprintf(b, "H %s\n", FossilEncode(d.H)) } if d.I != "" { fmt.Fprintf(b, "I %s\n", d.I) } for _, j := range d.J { if j.Value != "" { fmt.Fprintf(b, "J %s %s\n", FossilEncode(j.Name), j.Value) } else { fmt.Fprintf(b, "J %s\n", FossilEncode(j.Name)) } } if d.K != "" { fmt.Fprintf(b, "K %s\n", d.K) } if d.L != "" { fmt.Fprintf(b, "L %s\n", FossilEncode(d.L)) } if len(d.M) > 0 { sorted := make([]string, len(d.M)) copy(sorted, d.M) sort.Strings(sorted) for _, m := range sorted { fmt.Fprintf(b, "M %s\n", m) } } if d.N != "" { fmt.Fprintf(b, "N %s\n", d.N) } if len(d.P) > 0 { fmt.Fprintf(b, "P %s\n", strings.Join(d.P, " ")) } for _, q := range d.Q { prefix := "+" if q.IsBackout { prefix = "-" } if q.Baseline != "" { fmt.Fprintf(b, "Q %s%s %s\n", prefix, q.Target, q.Baseline) } else { fmt.Fprintf(b, "Q %s%s\n", prefix, q.Target) } } if d.R != "" { fmt.Fprintf(b, "R %s\n", d.R) } if len(d.T) > 0 { sorted := make([]TagCard, len(d.T)) copy(sorted, d.T) sort.Slice(sorted, func(i, j int) bool { ki := string(sorted[i].Type) + sorted[i].Name + sorted[i].UUID kj := string(sorted[j].Type) + sorted[j].Name + sorted[j].UUID return ki < kj }) for _, tag := range sorted { fmt.Fprintf(b, "T %c%s %s", tag.Type, tag.Name, tag.UUID) if tag.Value != "" { fmt.Fprintf(b, " %s", tag.Value) } b.WriteString("\n") } } if d.U != nil && *d.U != "" { fmt.Fprintf(b, "U %s\n", FossilEncode(*d.U)) } if len(d.W) > 0 { fmt.Fprintf(b, "W %d\n%s\n", len(d.W), d.W) } }