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

update: prioritise atomic units to prefixed units #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pRdm
Copy link

@pRdm pRdm commented Feb 2, 2018

Hi, first of all I'd like to thank you for making this fantastic gem. I've been trialing this gem for basic unit conversions, and it's worked out great for me so far.

Because I'm using this gem for basic unit parsing, I had the same issue as #16, where parsing 'ft' gave me back femto tonne instead of the desired unit foot. This PR is intended to change that so exact atomic unit matches are prioritised over prefixed matches.

Obviously this breaks the original parsing precedence of metric > rest, and may not be what you desire. Maybe we can add a configuration object for toggling the parsing precedence?

- introduced Unitwise::Expression::AtomicParser into Unitwise::Expression::Decomposer
- additional tests
- some refactoring to adhere to rubocop style recommendations
@bgentry
Copy link

bgentry commented Jun 27, 2018

This seems pretty reasonable. I'd wager that the number of people attempting to parse ft to foot is orders of magnitude higher than the number of people that actually want femto tonne 😄

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