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 dhall-json always pretty-print JSON? #716

Closed
Gabriella439 opened this issue Nov 27, 2018 · 14 comments · Fixed by #1373
Closed

Should dhall-json always pretty-print JSON? #716

Gabriella439 opened this issue Nov 27, 2018 · 14 comments · Fixed by #1373

Comments

@Gabriella439
Copy link
Collaborator

I was having difficulty thinking of a use case where a user wouldn't want to pretty-print the JSON output. I was wondering if we should pretty-print the output by default

@ocharles
Copy link
Member

ocharles commented Nov 28, 2018 via email

@Gabriella439
Copy link
Collaborator Author

I view pretty-printing as ancillary to jq's purpose. If anything, I think jq is an example of why we should do this since jq does not delegate responsibility for pretty printing to another tool

@ocharles
Copy link
Member

I don't understand. Jq doesn't care if your input is or isn't pretty printed. However, it is a tool that solves the general problem of "I want this JSON pretty printed". The purpose of dhall-to-json is just to convert Dhall to JSON. Personally I have no feeling either way on the matter though, technically either is an arbitrary choice

@joneshf
Copy link
Collaborator

joneshf commented Nov 29, 2018

Is the proposal to flip the boolean, or is the proposal to remove it and always pretty print?

@Gabriella439
Copy link
Collaborator Author

@joneshf: Remove it and always pretty print

Note that if I were to o this I'd probably roll this out in two phases:

  • Phase 1 - always pretty print and ignore the option
  • Phase 2 - remove the option

@justinwoo
Copy link
Contributor

Sorry to comment late if the decision has already been made, the one thing I care about is if the output has newlines or not. Having this be a flag would be useful for me, but otherwise I will use other tools to substitute newlines.

@Gabriella439
Copy link
Collaborator Author

@justinwoo: The decision hasn't been made, yet! 🙂

I can definitely preserve a newline-free option if you need one

@Profpatsch
Copy link
Member

@justinwoo Let me guess, so that it’s easier to use with ad-hoc perl scripts? ;)

@justinwoo
Copy link
Contributor

it's useful for when you work with newline-delimited JSON and other things in general, but yes, this would also help me use jq and perl

@Gabriella439
Copy link
Collaborator Author

So then the transition can be done in the following phases:

  • Add a --compact flag that enforces the current default of single-line output
  • Change the default to --pretty
  • Remove the --pretty flag

Gabriella439 added a commit that referenced this issue Dec 8, 2018
This is phase 1 of transitioning to pretty-printing by default, as
described here:

#716 (comment)

This flag allows users who prefer the compact format a smooth migration
path
Gabriella439 added a commit that referenced this issue Dec 9, 2018
This is phase 1 of transitioning to pretty-printing by default, as
described here:

#716 (comment)

This flag allows users who prefer the compact format a smooth migration
path
@Gabriella439
Copy link
Collaborator Author

Gabriella439 commented Sep 29, 2019

I'm adding a Hacktoberfest label to this to complete the next step of the transition to make --pretty the default behavior for dhall-to-json

@DKurilo
Copy link
Contributor

DKurilo commented Oct 2, 2019

Wanted to do something simple and found that:

  1. I can't build it with nix on my laptop (MacOS), because this line:
    https://github.com/dhall-lang/dhall-haskell/blob/master/default.nix#L20
    make nix trying to build website with linux version of ghcjs.
  2. As it's in README, I need to build modules with cabal while under nix-shell. As I see, I need v2 cabal command. Like cabal v2-run dhall-json -- --file ./examples/travis.yml to work with project.
    I can fix it, but I've never worked with this project before, so my question: Are these things really issues or did I understand it wrongly?

@sjakobi
Copy link
Collaborator

sjakobi commented Oct 2, 2019

2. As it's in README, I need to build modules with cabal while under nix-shell. As I see, I need v2 cabal command. Like cabal v2-run dhall-json -- --file ./examples/travis.yml to work with project.

You can alternatively use stack if you are more comfortable with that:

$ stack build dhall-json
$ stack exec -- dhall-to-json --file ./examples/travis.yml

I mostly use cabal, without the nix-shell.

@DKurilo
Copy link
Contributor

DKurilo commented Oct 2, 2019

I created pull request. Surely, it's very simple changes. Everything is already done. And I can create one more PR to remove --pretty right now.
But I think it's really breaking changes, because anyone who , like @justinwoo , as I understand, relies on new lines between multiple generated jsons will update dhall-to-json they will have unpredictable errors (maybe even in prod).

@mergify mergify bot closed this as completed in #1373 Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants