search

probablykasper / cpc

Text calculator with support for units and conversion
https://crates.io/crates/cpc
MIT License
121 stars 14 forks source link

Synonyms for operators #7

Closed djmattyg007 closed 3 years ago

djmattyg007 commented 3 years ago

Is it possible to support queries like this?

❯ ./cpc '500 divided by 4'
Lexing error: Invalid string: divided
probablykasper commented 3 years ago

Great idea, would like to add that

djmattyg007 commented 3 years ago

I've managed to add single-word synonyms for some of the operators, like "plus" and "minus". However, the multi-word synonyms (like "multiplied by" and "divided by") don't seem to work. I just get a message like this:

❯ target/release/cpc '5 divided by 2'
Lexing error: Invalid string: divided
probablykasper commented 3 years ago

The section where most words are lexed just deal with single-word keywords. Don't remember 100% how it works, but this is where two-word units are handled: https://github.com/probablykasper/cpc/blob/9bf7c69c038d5c747108e7e82bd3564532d28389/src/lexer.rs#L86-L90

djmattyg007 commented 3 years ago

Yeah I just came across that as your message came through :stuck_out_tongue: I also noticed that lexing of units such as "oil barrels" and "fluid ounces" don't seem to work because they aren't in that list.

djmattyg007 commented 3 years ago

Perhaps it needs to be reworked such that if we find a prefix of an operator, but haven't found an actually valid operator, we need to add the next word and try matching again.

I have no idea of how to actually implement that XD I'm brand new to rust.

probablykasper commented 3 years ago

Ah lol, no wonder you were confused.

Not really sure what would be the best way to go about it. The way it is now makes it nice and readable, but easier to add bugs.

An alternative would be checking for cubic, and then having a nested match statement with meter, centimeter etc. This kinda disconnects the two-word units from the others, and probably results in a few more lines, but harder to add bugs.

djmattyg007 commented 3 years ago

Yeah, that's not a bad idea. That would support arbitrary matching too.

I'll see what I can come up with.

djmattyg007 commented 3 years ago

I thought I'd made some decent progress with this, then I proceeded to spend a decent chunk of the next 24 hours fighting the borrow checker :(

I'm going to submit a draft PR to kinda demonstrate the approach I took, but I suspect it won't actually be viable for the final product.

probablykasper commented 3 years ago

That's rough :/ I'll make sure to check out the draft PR and see if I have any ideas

probablykasper commented 3 years ago

Added by #21 and #18