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

Implemented serialization using pretty print #19

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

barreyo
Copy link

@barreyo barreyo commented Mar 10, 2017

Implemented printing of a toml using pretty printing combinators.

Using the printer on the example file produces:

> ppTable toml
[database]
enabled = true
ports = [8001, 8001, 8002]
connection_max = 5000
server = "192.168.1.1"
[servers.alpha]
dc = "eqdc10"
ip = "10.0.0.1"
[servers.beta]
dc = "eqdc10"
country = "中国"
ip = "10.0.0.2"
[owner]
bio = "GitHub Cofounder & CEO
Likes tater tots and beer."
dob = 1979-05-27T07:32:00Z
name = "Tom Preston-Werner"
organization = "GitHub"
[servers1]
[clients]
data = [["gamma", "delta"], [1, 2]]
hosts = ["alpha", "omega"]
[[products]]
name = "Hammer"
sku = 738594937
[[products]]
color = "gray"
name = "Nail"
sku = 284758393
title = "TOML Example"

@barreyo
Copy link
Author

barreyo commented Mar 10, 2017

The code is a bit messy right now, but we are working on improving it. This fixes bug #17 .

@cies
Copy link
Owner

cies commented May 31, 2017

It looks good. I want to merge it, but still feel it lacks some sort of tests. Maybe not needed for "pretty printing", but some might use it for more then that (like generating TOML files). Would it be possible to add some basic tests for this? (most features of htoml are extensively tested)

@cies
Copy link
Owner

cies commented May 31, 2017

I also wonder if it is not better to ship this as a separate package htoml-pretty. This seems to be a common way to add a pretty printer to a lib, as not everyone needs this functionality.

@chshersh
Copy link

I would appreciate having pretty printer for toml data type. I don't think you need to put this into separate library. As for me, having in pretty-printing is very important. Consider next scenario: you have config in TOML format, you parse it, you modify it and then you need to write it back.

@cies
Copy link
Owner

cies commented Nov 17, 2017

Sorry all involved. I had you wait much longer then should have. And thanks @chshersh for chiming in. So 3 is a crowd and thus PP is now the most requested feature.

I want to pull the code in, but think it needs at least one test case. Like one complex TOML tile that gets parsed (internal representation A), PPed, that output reparsed (internal representation B). A and B need to be the same.

Bonus points for using QuickCheck to generate the initial internal state :)

TOML is quite heavily tested, and I think all code should be under some test for a library like this.

Final note: if we internally represent whitespace and comments then this feature will be more complete, but I'm ok with pulling the code in without that (it just needs a test).

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.

4 participants