.dhallb ergonomics

[sjakobi]

Working with tests for the binary encoding and reviewing changes to .dhallb files is currently tricky as the binary files are hard to read and GitHub can't show the diffs.

I think it would be better to include the JSON representations of the binaries. We could

1. Replace the .dhallb with .json files produced by cbor-tool dump --json. Implementations would then have to encode the JSON files to check their binary decoders.

2. Add the JSON files in addition to the .dhallb files and use CI to check their equivalence.

[f-f]

@sjakobi we used to have these files encoded in JSON, and we moved them to binary in #393, due to reasons detailed in #384 (TL;DR: encoding them in JSON doesn't work because JSON is imprecise)

To have decent ergonomics you should use cbor2diag as suggested by @philandstuff in #504 (I'd also recomment that snipped of git config)

[f-f]

Update: I added this recommendation to the README for the test suite in #657

[sjakobi]

Thanks for the pointers and the docs improvement, @f-f! 👍

I still think that it would be more ergonomic to include textual representations in this repo.

[f-f]

You're welcome 🙂

I wouldn't like to version another representation of these files, because:

• you can easily get them with the tool anyways
• it's hard enough to keep the .dhallb files up to date and correct, another representation that might drift (because they are not the source of truth) could only add maintenance burden

[sjakobi]

So how do you currently review PRs that change a .dhallb @f-f. Do you check each one out and git diff locally?

[f-f]

Do you check each one out and git diff locally?

@sjakobi yes, it's slightly uncomfortable (as e.g. I cannot review on mobile), but I prefer it to having generated files checked in

[sjakobi]

I think the confusing discussion in #655 is a good case in point why we need a textual representation of .dhallb files in this repo!

As the JSON representation has drawbacks as @f-f pointed out, the diagnostic format of cbor2diag that is described in https://tools.ietf.org/html/rfc7049#section-6 looks like a good fit.

[philandstuff]

Diagnostic format is nice but it can cause confusing diffs because the right hand side is indented at different levels depending on the full content of the left. This means you can get a lot of whitespace-only diffs when changing dhallb files.

(For what it's worth, I'm pretty neutral on this change. Overall I think that there's a minor benefit to having a human readable format checked in to the repo, but there's a minor drawback which is that my spec tests will need to call out to diag2cbor.rb; in dhall-golang's case, the tests already call cbor2diag.rb on failure to give a better error message.)

[Gabriella439]

I support this, because rom a code review perspective this will make it easier to catch bugs in the binary files. I also think there is a way to do this without requiring implementations to support the diagnostic format.

What we can do is something similar to what our CI currently does to ensure that the Prelude is linted and internally cached. Specifically, we would:

• Keep both .dhallb and .diag files in the repository
• Create a convenient script named ./scripts/generate-diag.sh (analogous to ./scripts/lint-prelude.sh) that would generate the correct .diag files from the .dhallb files using cbor2diag.rb
• Have CI verify that the script has no effect for the contents of a pull request (just like it does for lint-prelude.sh)

[Gabriella439]

Fix is up here: #659