Skip to content

Commit

Permalink
sync: fix null handling when updating filepaths/metadata (#501)
Browse files Browse the repository at this point in the history
Before this commit, when updating a `.meta/config.json` file that
contained an optional key with the value of `null`, configlet would
produce an `UnpackDefect`. For example, the below tries to update a
file that contains `"contributors": null`:

    $ git clone https://github.com/exercism/common-lisp
    $ cd common-lisp
    $ git checkout 4fc524e24a69
    $ configlet sync --metadata -e binary -u
    Checking exercises...
    Cloning https://github.com/exercism/problem-specifications/... success
    [warn] metadata: unsynced: binary
    sync the above metadata ([y]es/[n]o)? y
    options.nim(194)         get
    Error: unhandled exception: Can't obtain a value from a `none` [UnpackDefect]

`configlet fmt` removes optional key/value pairs when the value is
`null`, and only the Common Lisp and Julia tracks had such a value, but
`configlet sync -u` should still handle this situation properly.

The bug: our `pretty` proc serializes from a list of keys, and relied on
optional values being non-null. `configlet fmt` added only keys with
non-null values to that list, but `configlet sync` tried to preserve
null values to reduce noise in diffs.

Fix by ensuring that `pretty` in sync mode can no longer sometimes use
`get` with a `none`.

Fixes: #500
  • Loading branch information
ee7 authored Jan 27, 2022
1 parent 5419212 commit 4ccb2e3
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 4 deletions.
20 changes: 16 additions & 4 deletions src/sync/sync_common.nim
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,12 @@ func addArray(s: var string; key: string; val: openArray[string];
else:
s.add "[],"

func addNull(s: var string; key: string; indentLevel = 1) =
## Appends the pretty-printed JSON for a `key` and its null value to `s`.
s.addNewlineAndIndent(indentLevel)
escapeJson(key, s)
s.add ": null,"

func addString(s: var string; key, val: string; indentLevel = 1) =
## Appends the pretty-printed JSON for a `key` and its string `val` to `s`.
s.addNewlineAndIndent(indentLevel)
Expand Down Expand Up @@ -460,6 +466,12 @@ func keyOrderForFmt(e: ConceptExerciseConfig |
if e.custom.isSome() and e.custom.get().len > 0:
result.add eckCustom

template addValOrNull(key, f: untyped) =
if e.key.isSome():
result.f(&"{key}", e.key.get())
else:
result.addNull(&"{key}")

proc pretty*(e: ConceptExerciseConfig | PracticeExerciseConfig,
prettyMode: PrettyMode): string =
## Serializes `e` as pretty-printed JSON, using:
Expand Down Expand Up @@ -490,27 +502,27 @@ proc pretty*(e: ConceptExerciseConfig | PracticeExerciseConfig,
of eckAuthors:
result.addArray("authors", e.authors)
of eckContributors:
result.addArray("contributors", e.contributors.get())
addValOrNull(contributors, addArray)
of eckFiles:
result.addFiles(e.files, prettyMode)
of eckLanguageVersions:
result.addString("language_versions", e.language_versions)
of eckForkedFrom:
when e is ConceptExerciseConfig:
result.addArray("forked_from", e.forked_from.get())
addValOrNull(forked_from, addArray)
of eckIcon:
when e is ConceptExerciseConfig:
result.addString("icon", e.icon)
of eckTestRunner:
when e is PracticeExerciseConfig:
result.addBool("test_runner", e.test_runner.get())
addValOrNull(test_runner, addBool)
of eckBlurb:
result.addString("blurb", e.blurb)
of eckSource:
result.addString("source", e.source)
of eckSourceUrl:
result.addString("source_url", e.source_url)
of eckCustom:
result.addObject("custom", e.custom.get())
addValOrNull(custom, addObject)
result.removeComma()
result.add "\n}\n"
19 changes: 19 additions & 0 deletions tests/test_sync.nim
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,25 @@ proc testSyncCommon =
check:
exerciseConfig.pretty(pmSync) == expected

test "pretty: can use `pmSync` when an optional key has the value `null`":
let exerciseConfig = PracticeExerciseConfig(
originalKeyOrder: @[eckContributors],
contributors: none(seq[string])
)
const expected = """{
"authors": [],
"contributors": null,
"files": {
"solution": [],
"test": [],
"example": []
},
"blurb": ""
}
""".dedent(6)
check:
exerciseConfig.pretty(pmSync) == expected

proc stdlibSerialize(path: string): string =
var j = json.parseFile(path)
for key in j.keys():
Expand Down

0 comments on commit 4ccb2e3

Please sign in to comment.