Skip to content

Conversation

@ocharles
Copy link
Member

@ocharles ocharles commented May 16, 2019

This uses Data.Text.Prettyprint.Doc.removeTrailingWhitespace to remove
trailing whitespace from pretty printed documents.

In the process I also refactored this module a bit to remove duplication.

Fixes #183.

This uses Data.Text.Prettyprint.Doc.removeTrailingWhitespace to remove
trailing whitespace from pretty printed documents.

Fixes dhall-lang#183.
@ocharles
Copy link
Member Author

ocharles commented May 16, 2019

Note that this is not ready to be merged as it removes blank lines. Working with @quchen on seeing where the problem is.

prettyprinter-1.3.0 is now out with the fix 🎉

@ocharles
Copy link
Member Author

Looks like we need to wait until hnix supports prettyprinter-1.3.0 now.

Copy link
Collaborator

@Gabriella439 Gabriella439 left a comment

Choose a reason for hiding this comment

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

Feel free to jailbreak hnix if it's just a bounds failure that is blocking this. dhall-to-nix isn't Hackage-ready yet anyway (until hnix cuts a new Hackage release) since dhall-to-nix currently depends on a git revision of hnix

@ocharles
Copy link
Member Author

Ok, if you're happy with that I'll jailbreak 👍

@quchen
Copy link
Contributor

quchen commented May 21, 2019

I thought about which version number to bump, but the Pretty [a] instance does observably change output, so tests like tasty-golden are in danger of breaking.

@Gabriella439
Copy link
Collaborator

@quchen: Don't worry about it 🙂

@Gabriella439
Copy link
Collaborator

@ocharles: If you merge ocharles#3 into your branch it should at least fix the Nix build failures

@Gabriella439
Copy link
Collaborator

Actually, I noticed something odd while using this branch for unrelated reasons. It appears to mess up formatting and color codes.

For example, on master, the following expression:

{ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = 1, bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = 2, cccccccccccccccccccccccccccccccccc = 3 }

... is formatted as:

{ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa =
    1
, bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb =
    2
, cccccccccccccccccccccccccccccccccc =
    3
}

... with syntax highlighting. But on this branch the highlighting is gone and it's formatted as:

{ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa =
1
, bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb =
2
, cccccccccccccccccccccccccccccccccc =
3
}

You'll see similar behavior for unions, too.

@sjakobi
Copy link
Collaborator

sjakobi commented Oct 16, 2019

I've started working on a patch that will supersede this PR.

@sjakobi
Copy link
Collaborator

sjakobi commented Oct 16, 2019

@Gabriel439 The missing colors are probably due to the unAnnotateS in prettyExprSource.

The removal of leading whitespace seems to be bug in a prettyprinter: See quchen/prettyprinter#84.

@sjakobi sjakobi mentioned this pull request Oct 16, 2019
3 tasks
@sjakobi
Copy link
Collaborator

sjakobi commented Oct 18, 2019

Closing in favour of #1422.

@sjakobi sjakobi closed this Oct 18, 2019
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.

dhall-format inserts trailing whitespace

4 participants