Repository navigation
feat(model)!: render start as a string and share one enum codec - #251
Merged
Merged
Conversation
Closes #241 Closes #215 `Status` and `TaskType` each carried their own name map, `String`, `MarshalJSON` and `UnmarshalJSON`, written piece for piece the same way. They now share one generic `enumCodec[T ~int]`: the name map stays the single source of truth and the decode direction is derived from it, so the two cannot drift. `start` was the last enum the CLI shipped as a bare integer. It is now a named `model.Start` on both `Task` and `Project`, using the same codec, rendering `inbox`, `anytime` or `someday`. The ad hoc decode in `whenText` is gone, so the agent brief and JSON take their list names from one place. `startBucket` stays an integer: `1` is the app's This Evening section and `0` is everything else, and only the first has a name in Things' own vocabulary. BREAKING CHANGE: `start` in JSON output is now the string `inbox`, `anytime` or `someday` rather than Things' raw integer `0`, `1` or `2`. A caller matching on `.start==2` has to become `.start=="someday"`. The legacy integer still decodes on input. Plain text output is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #241
Closes #215
What changed
StatusandTaskTypeeach carried their own name map,String,MarshalJSONandUnmarshalJSON, written piece for piece the same way. They now share one genericenumCodec[T ~int]ininternal/model. The name map stays the single source of truth and the decode direction is derived from it at construction, so the two directions cannot drift. Behaviour is unchanged for both types, down to the error text.startwas the last enum the CLI shipped as a bare integer. It is now a namedmodel.StartonTaskand onProject, using the same codec, so it rendersinbox,anytimeorsomedayin JSON and still accepts the legacy integer on input. The ad hocswitchinwhenTextis gone: the agent brief and JSON now take their list names from one place.Breaking change
startin every JSON output goes from an integer to a string. 0.8.0 already carries the same kind of change fortype, so this belongs in the same release note."start": 0"start": "inbox""start": 1"start": "anytime""start": 2"start": "someday"A filter matching on the integer has to be updated:
jq '.[] | select(.start==2)'becomesjq '.[] | select(.start=="someday")'. An unrecognised raw code is still emitted as its integer rather than collapsing to a lossy"unknown", so a value the CLI does not know round-trips.UnmarshalJSONaccepts the name or the legacy integer, and treats a JSON null as a no-op, matchingtypeandstatus.The field appears on to-do rows and, since #204, on
things projectsrows, so both surfaces change together.The
startBucketdecisionstartBucketstays an integer, and the reason is in the data rather than in caution. Every row in the live database holds0or1, and1only ever appears on a datedanytimeto-do: it is the Evening split within a scheduled day, the app's This Evening section.Only one of those two values has a name. Things' own vocabulary has
evening, which the CLI already exposes as--when evening, and it has no word at all for the other side. Naming the pair would have meant inventing a token for0and then asserting it on every row that is not an evening row, in a public contract that would take another breaking change to undo.0says nothing, which is what it means. The field now carries a doc comment onmodel.Tasksaying exactly this.If it ever does want names, the shared codec makes it a five-line change.
Verification
Beyond the existing suites, which stay untouched and green:
origin/mainacross twenty-one surfaces, checked by diffing the two binaries against the live database: nine list views with and without--include-completed, plusprojects,areasandtags.anytimeandsomedaypaths throughwhenText.start, on every list view and onprojects. Nothing else moved.TestRunJSONRendersStartAsStringasserts against raw JSON rather than an unmarshalledmodel.Task, the way feat(output)!: render type as a string in JSON #214 did fortype:Start.UnmarshalJSONstill accepts the legacy integer, so decoding would keep passing even if the encoder regressed. A regex fails the test if a bare number reaches thestartfield.Startas a raw int fails all six subtests of the new raw-JSON test; changing the codec's fallback name fails nine tests across three packages; wiring theStartcodec with another type's name fails the newTestEnumCodecErrorsNameTheirOwnType, which exists because nothing else would notice that mistake.TestStatusStringand the zero-value round-trip case that the #214 review noted were missing are both added, which is the rest of #215.One deliberate behaviour change beyond
startwhenTextused to fall through to"anytime"for anystartit did not recognise, and now renders"unknown", matching howtypeandstatusrender an unknown code.internal/dbcoalesces a NULLstartto0and Things only ever writes0,1or2, so this is unreachable against a real database;"unknown"is the more honest answer if it ever were reached.Review
/code-review --fixat high effort raised two points, both low. One was an ambiguous "both of these" incommands.mdsitting two paragraphs after a sentence naming three fields, which could have led a reader to rewrite a working.status=="open"filter as part of the migration; that is fixed, and the sentence now namestypeandstart. The other was a comment onwhenTextclaiming the brief and the JSON field "cannot drift apart", which overstates it: for an unrecognised code JSON keeps the integer while the brief says"unknown". The comment now says so.The review separately cleared the scan path (
database/sqlhandles a namedintthrough the same reflection pathStatusalready relied on), confirmed no non-test code outsideinternal/modelandinternal/outputreads eitherStartfield, confirmed"start"is emitted from exactly one place so there is no second encoder to keep in step, and confirmed the on-disk cache stores UUIDs only, so there is no stale JSON to migrate.Not touched
README.mdshows no JSONstartvalue anywhere, and documents neitherstatusnortypeas string enums, so there was nothing in it to update. It stays the short version and points at the site. Thet.start = 0/1/2literals ininternal/db/tasks.goare left alone: that file belongs to #240, and #238 is in it.