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

Lazy parser #37

Merged
merged 10 commits into from
Aug 9, 2018
Merged

Lazy parser #37

merged 10 commits into from
Aug 9, 2018

Conversation

andrew-lei
Copy link
Collaborator

@andrew-lei andrew-lei commented Aug 8, 2018

Closes #9.

@andrew-lei
Copy link
Collaborator Author

Not clear to me whether the failed job (stack 8.2.2) is due to the laziness test or if it just happened to time out.
Also, I haven't incremented the version number, and 'containers' and 'parsec' are now redundant.

@andrew-lei
Copy link
Collaborator Author

Figured out how to rerun the job. Looks like it was just bad luck.

@cdepillabout
Copy link
Owner

cdepillabout commented Aug 8, 2018

Thanks again for implementing this! The code looks great.

Also thanks for investigating the travis job.

You want to go ahead and remove the containers and parsec dependencies, update the version number, and make a release? (If not, no problem, I can always do it.)

I don't think this PR changes the types of any user-facing functions, so we could probably just update the minor version number. However, this lazy functionality is really great, and a big change to pretty-simple, so I'd even be fine with making the version something like 3.0.0.0. (I'd also like to make a blog post introducing this new version of pretty-simple, so it would sound better if it was bumped to 3.0.0.0, however this is a bad reason to decide the version of a library!)

@cdepillabout
Copy link
Owner

Also, this is from #9 (comment):

As I've mentioned, I think it would work well as part of a post-processing step that would allow a user to perform more general sorts of manipulation on the intermediate bits. So the post-processor should be a function either [Output] -> [Output] or [Expr] -> [Expr]. I'm leaning towards the latter because I think it would allow for more general manipulation of the structure (for instance, to hide everything in certain Parens).

I was thinking one way to do this would be to perhaps have a constructor that contains the parsed value and either the type information (string literal, number, datetime, etc.) or the colour.

Did you want to create a new issue about this? Unless you're planning on doing this right away, it might help someone else looking to implement this.

@cdepillabout
Copy link
Owner

Sorry, one more thing. I just opened #38 adding a test that unicode is printed out correctly.

Could you either merge that into this PR, or just merge #38 into master after merging this PR?

@andrew-lei
Copy link
Collaborator Author

OK. I have bumped the version number to 2.2.0.0 and updated the changelog. I will open up that comment as another issue and I will merge as soon as Travis has finished.

@andrew-lei andrew-lei merged commit 51b3645 into cdepillabout:master Aug 9, 2018
@andrew-lei
Copy link
Collaborator Author

OK, I managed to upload it to Hackage. The documentation isn't appearing, though, but I'm assuming that it takes a moment to generate?

@cdepillabout
Copy link
Owner

@andrew-lei Thanks a lot for uploading this to Hackage!

The documentation usually takes a while to appear. It looks like has been created:

http://hackage.haskell.org/package/pretty-simple

Also, in the future, when you make a release, would you be able to create a tag and push it to github? I'll go ahead and run the following commands at the current master branch:

$ git tag v2.2.0.0 -m 'release version 2.2.0.0'
$ git push --tag

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.

2 participants