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

Should the default dhall command emit the type to stderr? #539

Closed
Gabriella439 opened this issue Aug 8, 2018 · 7 comments
Closed

Should the default dhall command emit the type to stderr? #539

Gabriella439 opened this issue Aug 8, 2018 · 7 comments

Comments

@Gabriella439
Copy link
Collaborator

One issue that I've begun to run into recently is the fact that the default dhall command (i.e. no subcommand) reserves stderr for printing the inferred type of the expression. This prevents the executable from emitting useful information to stderr for successful runs.

For example, the next release of the standard will say:

An implementation SHOULD warn the user if the interpreter is unable to cache the expression due to the environment variables being unset or the filesystem paths not being readable or writeable.

The natural place to put that warning would be stderr but that would cause issues for people expecting stderr to contain the type.

Similarly, at some point we might want to provide download progress indicators for remote imports and the natural place for that would also be stderr.

Currently, users can use the dhall type subcommand to programmatically retrieve the inferred type of an expression, so there is less of a need to include the inferred type in the default command's output.

So I'm going to propose a few solutions to this problem so that we can include useful diagnostic information in stderr:

  • Omit the inferred type from stderr entirely for the default command (i.e. reserve stderr exclusively for debug output and reserve dhall type as the exclusive way to infer the type)
  • Keep the inferred type, but comment out other debug output sent to stderr so that one can still capture stderr as a valid Dhall type
  • Keep the inferred type, but don't guarantee that stderr still reflects a valid type if there is debug output

My inclination is for the second option (comment out debug output), but I'm open to other ideas

@Profpatsch
Copy link
Member

Profpatsch commented Aug 9, 2018

I think we should never abuse stderr for carrying in-band data (as compared to meta data, like warnings or errors). Users should never use any data on stderr for further processing (except maybe formatting for logging).

If a user wants to prevent having to call the interpreter twice, there’s other methods, like having a --type-output argument which the user can pass a FIFO to. Or maybe even a file handle.

@ocharles
Copy link
Member

ocharles commented Aug 9, 2018

I find the current behaviour inconvenient as I'm calling dhall during Nix evalutaion (with import-from-derivation). The current behavior means that during evaluation Dhall types get printed out. However, I can't redirect stderr to /dev/null because then I would actually lose error messages when there is a problem.

@sellout
Copy link
Collaborator

sellout commented Aug 9, 2018

I vote for option one (no types on stderr). What if some option like @Profpatsch suggests outputs the normalized form with an explicit type annotation, so that the stdout is still valid Dhall?

@Gabriella439
Copy link
Collaborator Author

Alright, then we'll go with no types on stderr. Let me also see if I can add an easier way to obtain the type besides dhall resolve | dhall type

@f-f
Copy link
Member

f-f commented Aug 9, 2018

@Gabriel439 I'd +1 as well on the --type-output flag as @Profpatsch suggested

@Gabriella439
Copy link
Collaborator Author

I really like @sellout's suggestion of including the type annotation in stdout. I think the main question on my mind is whether or not the type annotation should be present or absent by default. Either way there would be some option to toggle the type annotation.

@Gabriella439
Copy link
Collaborator Author

I think I'll tentatively go with omitting the type annotation by default and allowing the user to opt in using an --annotate switch

Gabriella439 added a commit that referenced this issue Aug 11, 2018
Fixes #539

The default `dhall` command no longer outputs types to `stderr`.
Instead, the user can opt into a type annotation using a newly added
`--annotation` switch.
Gabriella439 added a commit that referenced this issue Aug 12, 2018
Fixes #539

The default `dhall` command no longer outputs types to `stderr`.
Instead, the user can opt into a type annotation using a newly added
`--annotation` switch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants