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

Cannot register unit #51

Open
pinkynrg opened this issue Aug 10, 2017 · 9 comments
Open

Cannot register unit #51

pinkynrg opened this issue Aug 10, 2017 · 9 comments

Comments

@pinkynrg
Copy link

pinkynrg commented Aug 10, 2017

I'm trying to register a unit called 'mma'.

Unitwise.register(
  names: "mma",
  symbol: "mma",
  primary_code: "mma",
  secondary_code: "MMA",
  scale: {
    value: 12.0,
    unit_code: 'm'
  },
  property: '...'
)

Once i do:

 Unitwise::Atom.find("mma")

I get: => #<Unitwise::Atom names=["xxx"], primary_code="mma", secondary_code="MMA", symbol="XXX", scale=#<Unitwise::Scale value=1 unit=m>, classification=nil, property="xxx", metric=false, special=false, arbitrary=false, dim="L">

but when I do this:

Unitwise(1,"mma")

I get: Unitwise::ExpressionError: Could not evaluate 'mma'.

Same thing by using the secondary_code: Unitwise::ExpressionError: Could not evaluate 'MMA'.

@pinkynrg
Copy link
Author

Debugging I get this error:
*** Parslet::ParseFailed Exception: Failed to match sequence (left:(GROUP / TERM)? (OPERATOR right:EXPRESSION)?) at line 1 char 3.

@pinkynrg
Copy link
Author

pinkynrg commented Aug 10, 2017

I have reason to believe this is the rule giving issues:

rule (:term) do
  ((factor >> simpleton | simpleton | factor) >>
  exponent.maybe >> annotation.maybe).as(:term)
end

This is proven by the fact that if I change the parslet root to be simpleton mma is not recognized - with also a few more testing match, like m (meter) or mm (millimeter).

@pinkynrg
Copy link
Author

pinkynrg commented Aug 11, 2017

I think it might be an issue with parslet
kschiess/parslet#181

@pinkynrg
Copy link
Author

This is was I found:

Consider that the following rule is not a leaf of the parsing tree so if I pick wrong i'm done, considered the OR "|" sign: for details kschiess/parslet#181):

 rule (:simpleton) do
   atom | prefix >> metric_atom
 end

atom can match any measurement_unit (atoms) - OR - a prefix and any metric measurement unit.

atoms right now are:

  • m (metric)
  • mma (non metric)

prefix are:

  • m (milli)

This is the outcome:

mma: is an atom, match ok
m: is an atom, match ok
mm: is a prefix + a metric atom, doesn't match

this is because the rule matches the first "m" of "mm" as it was an atom (meter), not a prefix (milli) and since it cannot come back it throw me exception later on.

I tried also to flip the 2 parts of the rule as follows:

 rule (:simpleton) do
   prefix >> metric_atom | atom
 end

it this case:

m: since it has no prefix, go to second part of the rule and matches
mm: it has prefix and it matches in the first part of the rule
mma: match as it was "mm" and then it fails

@pinkynrg
Copy link
Author

Solved by doing the following:

rule (:simpleton) do
  prefix.as(:prefix) >> metric_atom.as(:atom) >> simpleton_stop | atom.as(:atom) >> simpleton_stop
end

rule (:simpleton_stop) do
  match['(\.|\/|\-|\d)'].present? | any.absent?
end

Let me know what you think!

@joshwlewis
Copy link
Owner

Hmm, I see. Thanks for investigating. This sounds like the same issue described in #26.

Do all the tests pass with your proposed solution? My guess is that the simpleton_stop rule is consuming characters we don't want to consume (like dots, slashes, or exponents).

@pinkynrg
Copy link
Author

The 2 methods absent and present are not consuming.

Sent from my Motorola XT1585 using FastHub

@joshwlewis
Copy link
Owner

Ok, cool. I think this would be good to bring in. Can you open a PR with these changes, but make sure that the simpleton_stop rule includes all the characters listed as exceptions in section 2.1 here (e.g. brackets, "+", "=")?

@pinkynrg
Copy link
Author

Sure absolutely. I'm glad you find the fix worth it.

Sent from my Motorola XT1585 using FastHub

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

No branches or pull requests

2 participants