Decide whether frontmatter.UpdateListField goes, now that nothing calls it
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2
- Forks
- 0
- Avg merge
- 2h 8m
- Merged PRs (30d)
- 49
Description
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'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/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 |
readsBackInSeqAs (:269), elementNodeFor (:239), seqNodeFor |
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.mdhad to be amended in #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/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.- #154 (
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 #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.- Amend
_plans/040, which records the keep decision and its reasoning.
Related
- #151 / #156 — where the caller went, and where the keep decision was recorded.
- #154 — check whether
diffwants a metadata write path before deleting. - One smaller item of the same kind, worth resolving in the same pass:
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by checking #154's design, then read internal/frontmatter/frontmatter.go around UpdateListField and existingSeqIsFlow. Run the frontmatter tests, especially TestUpdateListFieldKeepsTheStyle, TestRenderListField, and TestBlockListSurvivesNormalize. Done means the keep-or-delete decision is implemented consistently in the code, tests, docs/confluence/labels.md, C2, and _plans/040.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- cli
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100