Skip to content

Re-export main API from single module and remove Text.Parsing prefix per #78 #89

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

Merged
merged 8 commits into from
Mar 8, 2022

Conversation

chtenb
Copy link
Member

@chtenb chtenb commented Mar 7, 2022

Fix #78


Checklist:

  • Added the change to the changelog's "Unreleased" section with a link to this PR and your username
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation in the README and/or documentation directory
  • Added a test for the contribution (if applicable)

@chtenb chtenb requested a review from thomashoneyman March 7, 2022 20:13
@thomashoneyman thomashoneyman added the type: breaking change A change that requires a major version bump. label Mar 7, 2022
@@ -1,6 +1,6 @@
-- | This module defines the `Parser` type of string parsers, and its instances.

module Text.Parsing.StringParser where
module StringParser.Common where
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be StringParser.Parser since it specifically deals with the parser type?

Copy link
Member Author

Choose a reason for hiding this comment

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

I named it Common because it contains more than just the Parser type, namely all the things that the CodeUnits parser and the CodePoints parser share but are not in Combinators.

But you have a lot more knowledge about naming conventions in the ecosystem, so should it be Parser instead? You tell me 😄

Copy link
Contributor

@thomashoneyman thomashoneyman Mar 8, 2022

Choose a reason for hiding this comment

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

I may be misunderstanding you; all I see in this file are the parser type and instances, plus runParser and unParser for running the parser, fail for setting an error message, and printParserError for unpacking the error message.

namely all the things that the CodeUnits parser and the CodePoints parser share but are not in Combinators.

I guess my question is — what is being shared here beyond the Parser type and helpers for its error messages? If it's nothing, then I think StringParser.Parser is a more informative module name. If it's more than that, then Common makes more sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

what is being shared here beyond the Parser type and helpers for its error messages?

That's pretty much it as you can see. So it's decided then :)

@chtenb chtenb merged commit 6397fae into main Mar 8, 2022
@chtenb chtenb deleted the exports branch March 8, 2022 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: breaking change A change that requires a major version bump.
Development

Successfully merging this pull request may close these issues.

Remove Text.Parsing module prefix
2 participants