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

Combine ingredients in shopping lists #71

Merged
merged 8 commits into from
Sep 2, 2018

Conversation

langston-barrett
Copy link
Collaborator

Reifying units to a type

  • This leads to a consistent internal representation. When dealing with units, we remove ambiguity at parse-time and never have to deal with synonyms internally.
  • This has the disadvantage that units aren't presented exactly as the user entered them (arguably an advantage).
  • It leads to more compile-time guarantees, when dealing with units GHC can now make sure our cases are exhaustive.

Combining ingredients

This is the goal of the reification. If you have two units with the same names and units, say:

  • 1 (no unit) onion, large
  • 2 (no unit) onions, medium
    It will be displayed in a shopping list as:
  • 3 onions, 1 large, 2 medium
    So it is even smart about combining attributes in a way that the user can recover all the original data 😄

Reorganization and cleanup

  • AddCLI is definitely not a library module, it defines a UI
  • Remove unnecessary modules from Cabal targets
  • Remove doctests (you can just run cabal doctest, which will do everything this code currently does automatically)

This leads to a consistent internal representation. When dealing with units, we
remove ambiguity at parse-time and never have to deal with synonyms internally.

This has the disadvantage that units aren't presented exactly as the user
entered them (arguably an advantage).

It leads to more compile-time guarantees, when dealing with units GHC can now
make sure our cases are exhaustive.
They can't find their dependent modules. Not sure why.
Users can just run "cabal doctest"
@langston-barrett
Copy link
Collaborator Author

Note that this introduces a backwards-incompatible change: units in already-defined recipes files will have to be modified.

app/herms.hs Outdated
@@ -378,7 +377,7 @@ servingP t = option auto
<> short Str.servingShort
<> help (t Str.servingDesc)
<> showDefault
<> value 0
<> value 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, I didn't meant to change this.

@langston-barrett
Copy link
Collaborator Author

This isn't quite ready:

  • breaking changes should be noted in the documentation
  • the edit window needs to use showUnit, not show

@LuxMiranda
Copy link
Owner

Wow! This is quite an update!!! I'm loving it! Is this ready to be merged?

@langston-barrett
Copy link
Collaborator Author

Not quite yet! I will finish it soon, likely tomorrow. All those changes should be quite small (~15 lines total), so feel free to give it a review.

@langston-barrett langston-barrett merged commit f1c8b3f into LuxMiranda:master Sep 2, 2018
@langston-barrett langston-barrett deleted the ingredient-set branch September 2, 2018 00:39
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