Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable --records-strict by default for {json,yaml}-to-dhall #1181

Merged
merged 3 commits into from
Aug 2, 2019

Conversation

Gabriella439
Copy link
Collaborator

This adds a new --records-loose flag and changes the default behavior
to --records-strict

The rationale for this change is so that this default behavior helps
more when users are still experimenting with the appropriate type to
use to decode a large and messy YAML file.

For example, suppose a user is trying to translate the following YAML
to Dhall:

foo: 1
bar: true

... and they start with a empty Dhall type which they plan to fix in
response to error messages. They'd get a surprising success:

$ yaml-to-dhall '{}' < ./example.yaml
{=}

... because the old default behavior is to ignore extra fields in the
YAML. However, if we're strict by default then users don't have to
apply the --records-strict flag to get feedback on what they need
to change.

This adds a new `--records-loose` flag and changes the default behavior
to `--records-strict`

The rationale for this change is so that this default behavior helps
more when users are still experimenting with the appropriate type to
use to decode a large and messy YAML file.

For example, suppose a user is trying to translate the following YAML
to Dhall:

```yaml
foo: 1
bar: true
```

... and they start with a empty Dhall type which they plan to fix in
response to error messages.  They'd get a surprising success:

```bash
$ yaml-to-dhall '{}' < ./example.yaml
{=}
```

... because the old default behavior is to ignore extra fields in the
YAML.  However, if we're strict by default then users don't have to
apply the `--records-strict` flag to get feedback on what they need
to change.
@sjakobi
Copy link
Collaborator

sjakobi commented Aug 1, 2019

I was wondering what the error message would look like:

$ stack exec -- yaml-to-dhall '{}' < example.yaml 


Error: Key(s) foo, bar present in the YAML object but not in the expected Dhall record type. This is not allowed unless you enable the --records-loose flag:

Expected Dhall type:
{}

YAML:
foo: 1
bar: true

I noticed that the message Error: Key(s) … would get longer and longer with each key. It might be better to list the unexpected keys after the error message.

We probably also shouldn't include the full input file in the error message.

👍 for the new default though!

@Gabriella439
Copy link
Collaborator Author

@sjakobi: Yeah, I plan to make further improvements to the error message here. I'm just fixing one issue at a time

@mergify mergify bot merged commit 3abef4e into master Aug 2, 2019
@mergify mergify bot deleted the gabriel/records_strict branch August 2, 2019 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants