You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
frontmatter.UpdateListField has had no production caller since #151 removed markfluence fix. Decide whether it goes, and with it the block-style-preservation contract it is the only way to reach.
How it got here
cmd/fix/fix.go:244 was its only non-test caller — git grep UpdateListField main~ confirms — because fix was the one verb that wrote a labels: list back into a file. Nothing else writes a frontmatter sequence surgically:
create's writeBackFrontmatter (cmd/create/create.go:1132) writes five scalar fields through UpdateField, and persistToManifest documents that "labels is deliberately not written" — writing them would rewrite them, since labels.Set carries normalized names.
update writes no files at all.
read/export go through Render, which builds a block from scratch rather than editing one.
PR #156 flagged this and kept the function deliberately, on the grounds that internal/frontmatter owns the frontmatter dialect rather than serving its current callers, and that deleting adjacent machinery mid-removal is how a removal grows a regression. This issue is that decision, taken on its own.
What is actually dead, precisely
The review that found this described the whole sequence-writing path as dead, which overstates it. The accurate picture:
status
UpdateListField (frontmatter.go:415)
dead — no production caller
existingSeqIsFlow (:467)
effectively dead — still called on every UpdateField at :441, but its return value is only consumed when f.List != nil, so no scalar write can observe it
live, via Render — read/export emit labels: through pagedoc.RenderFrontmatter
So the thing genuinely lost by deleting is narrower than "sequence writing": it is the only path that writes a sequence in block style.Render always emits flow (labels: [howto, runbook], pinned by TestRenderListField and TestEmptyListIsWrittenAsFlowEmpty). UpdateListField is the sole reader of an existing sequence's IsFlowStyle, and therefore the only code that can keep a block list a block list.
That contract has a reason worth not losing by accident: a set large enough to be written as a block list is exactly the set whose flow spelling is an unreadable single line. It is pinned by TestUpdateListFieldKeepsTheStyle (frontmatter_test.go:652) and, indirectly, TestBlockListSurvivesNormalize (:682) — note the latter covers Normalize, which is live via create, so block lists surviving a reorder is not at risk either way.
The case for deleting
Unreferenced exported API is a maintenance claim nobody is making good on. The contract can regress and no command will notice; only internal/frontmatter's own tests will.
docs/confluence/labels.md had to be amended in Remove markfluence fix #156 to say outright that no command exercises this any more, which is a docs smell pointing straight at the code.
markfluence is unreleased, so there is no external consumer and no deprecation to run.
It is the honest reading of the codebase's own discipline: CLAUDE.md treats the code as self-documenting, and an exported function with no caller documents a capability the tool does not have.
The case for keeping
internal/frontmatter is a library for the frontmatter dialect, not a helper sized to today's callers. Every rule in it was found by probing goccy, and re-deriving the block-style rule later means re-finding the same trap.
UpdateListField is Render's surgical counterpart. Deleting it leaves the package able to build a sequence but not edit one, which is an odd shape for a package whose whole job is reading and writing this block.
UpdateListField and existingSeqIsFlow, plus the flow parameter threaded through valueWithComment/seqNodeFor/elementNodeFor/readsBackInSeqAs — which then always writes flow and can lose the parameter entirely. That simplification is most of the value.
TestUpdateListFieldKeepsTheStyle goes; TestRenderListField and TestBlockListSurvivesNormalize stay.
**C2** (frontmatter-is-valid-yaml`) needs re-reading rather than assumed unaffected. It says a value may be "a sequence (in either YAML style) whose every element is one" — that is about reading, which is unchanged, but the guarantee's discussion should not be left implying markfluence can write either style when it can only write flow.
docs/confluence/labels.md's block-style bullet should move from "no command exercises this" to describing reading only.
Amend _plans/040, which records the keep decision and its reasoning.
One smaller item of the same kind, worth resolving in the same pass: internal/labels's exported Field const (labels.go:42) now has no reference outside its own package. It is used at three internal sites so it is not dead, but it is no longer part of anyone's API and could be unexported.
frontmatter.UpdateListFieldhas had no production caller since #151 removedmarkfluence fix. Decide whether it goes, and with it the block-style-preservation contract it is the only way to reach.How it got here
cmd/fix/fix.go:244was its only non-test caller —git grep UpdateListField main~confirms — becausefixwas the one verb that wrote alabels:list back into a file. Nothing else writes a frontmatter sequence surgically:create'swriteBackFrontmatter(cmd/create/create.go:1132) writes five scalar fields throughUpdateField, andpersistToManifestdocuments that "labels is deliberately not written" — writing them would rewrite them, sincelabels.Setcarries normalized names.updatewrites no files at all.read/exportgo throughRender, which builds a block from scratch rather than editing one.PR #156 flagged this and kept the function deliberately, on the grounds that
internal/frontmatterowns the frontmatter dialect rather than serving its current callers, and that deleting adjacent machinery mid-removal is how a removal grows a regression. This issue is that decision, taken on its own.What is actually dead, precisely
The review that found this described the whole sequence-writing path as dead, which overstates it. The accurate picture:
UpdateListField(frontmatter.go:415)existingSeqIsFlow(:467)UpdateFieldat:441, but its return value is only consumed whenf.List != nil, so no scalar write can observe itreadsBackInSeqAs(:269),elementNodeFor(:239),seqNodeForRender—read/exportemitlabels:throughpagedoc.RenderFrontmatterSo the thing genuinely lost by deleting is narrower than "sequence writing": it is the only path that writes a sequence in block style.
Renderalways emits flow (labels: [howto, runbook], pinned byTestRenderListFieldandTestEmptyListIsWrittenAsFlowEmpty).UpdateListFieldis the sole reader of an existing sequence'sIsFlowStyle, and therefore the only code that can keep a block list a block list.That contract has a reason worth not losing by accident: a set large enough to be written as a block list is exactly the set whose flow spelling is an unreadable single line. It is pinned by
TestUpdateListFieldKeepsTheStyle(frontmatter_test.go:652) and, indirectly,TestBlockListSurvivesNormalize(:682) — note the latter coversNormalize, which is live viacreate, so block lists surviving a reorder is not at risk either way.The case for deleting
internal/frontmatter's own tests will.docs/confluence/labels.mdhad to be amended in Remove markfluence fix #156 to say outright that no command exercises this any more, which is a docs smell pointing straight at the code.The case for keeping
internal/frontmatteris a library for the frontmatter dialect, not a helper sized to today's callers. Every rule in it was found by probing goccy, and re-deriving the block-style rule later means re-finding the same trap.UpdateListFieldisRender's surgical counterpart. Deleting it leaves the package able to build a sequence but not edit one, which is an odd shape for a package whose whole job is reading and writing this block.markfluence diff) may want it back.diffcompares frontmatter values, and if anything is ever added that reconciles one, the block-style contract is immediately load-bearing again. Worth checking Add a diff subcommand: show what differs between a page and its local markdown #154's design before deleting.If it is deleted
UpdateListFieldandexistingSeqIsFlow, plus theflowparameter threaded throughvalueWithComment/seqNodeFor/elementNodeFor/readsBackInSeqAs— which then always writes flow and can lose the parameter entirely. That simplification is most of the value.TestUpdateListFieldKeepsTheStylegoes;TestRenderListFieldandTestBlockListSurvivesNormalizestay.C2** (frontmatter-is-valid-yaml`) needs re-reading rather than assumed unaffected. It says a value may be "a sequence (in either YAML style) whose every element is one" — that is about reading, which is unchanged, but the guarantee's discussion should not be left implying markfluence can write either style when it can only write flow.docs/confluence/labels.md's block-style bullet should move from "no command exercises this" to describing reading only._plans/040, which records the keep decision and its reasoning.Related
diffwants a metadata write path before deleting.internal/labels's exportedFieldconst (labels.go:42) now has no reference outside its own package. It is used at three internal sites so it is not dead, but it is no longer part of anyone's API and could be unexported.