Skip to content

Conversation

@cdepillabout
Copy link
Collaborator

@cdepillabout cdepillabout commented Feb 2, 2021

Description of the change

Dhall-1.38 has been released: https://hackage.haskell.org/package/dhall-1.38.0/

This has a change in the input format detection in the Dhall.Format.Format datatype: https://hackage.haskell.org/package/dhall-1.38.0/changelog

  • dhall format will now preserve the character set of the formatted file by default. In other words, if the file uses ASCII punctuation then dhall format will format the file using ASCII punctuation.
  • If the file contains both ASCII and Unicode punctuation it will prefer Unicode by default
  • This is a breaking change because the Lam / Pi / Combine / CombineTypes, and Prefer constructors now take an additional argument to record which character set was used
  • Version 1.37.1:

    data Format = Format
      { chosenCharacterSet :: CharacterSet
      ,  ...
      }
  • Version 1.38.0:

    data Format = Format
      { chosenCharacterSet :: Maybe CharacterSet
      ,  ...
      }

The spago codebase is passing Dhall.Pretty.ASCII (which is a CharacterSet) to all the pretty-printing functions. This PR updates the code to pass Just ASCII, which causes spago to render all dhall files in ASCII (even if the input dhall code uses unicode symbols).

I did this because passing Nothing (which should cause dhall format to use ASCII or Unicode based on the input file) causes one of the tests to fail. However, at some point it might make sense to change this test and have spago also output either ascii or unicode depending on the format of the input dhall file.

Checklist:

  • Added the change to the "Unreleased" section of the changelog
  • Added some example of the new feature to the README
  • Added a test for the contribution (if applicable)

P.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 😊

@cdepillabout cdepillabout marked this pull request as ready for review February 2, 2021 05:59
Copy link
Member

@f-f f-f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fantastic, thank you 🙂

@f-f f-f merged commit a8e61fc into purescript:master Feb 2, 2021
@cdepillabout cdepillabout deleted the bump-dhall-1.38 branch February 2, 2021 08:11
@cdepillabout cdepillabout changed the title Bump to dhall-1.38.0. Let dhall format pick the output encoding based on the input Bump to dhall-1.38.0 Feb 2, 2021
@toastal
Copy link
Contributor

toastal commented Feb 3, 2021

Personally, I'd like to see that Nothing because if I use spago upgrade-set right now, my files get reformatted from what our project/formatter are set to.

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

Successfully merging this pull request may close these issues.

3 participants