search

goldfirere / units

The home of the units Haskell package
95 stars 19 forks source link

Spin-out parser as a separate package #36

Closed adamgundry closed 9 years ago

adamgundry commented 9 years ago

The parser for unit expressions in Data.Metrology.Parser.Internal is very nice, and I think it could be useful independently from units. (Full disclosure: I'm writing my own units of measure library, based on experiments with GHC typechecker plugins, and I'd much rather steal your parser than write my own!) Would you be receptive to a pull request if I create a tiny package (perhaps units-parser) and adapt units to work with it instead? My intention would be that all the TH-related functionality remain in units:Data.Metrology.Parser and only parseUnit and its dependencies move.

At the same time, I'd like to introduce a small generalization: rather than requiring a finite map from strings to units, we could take type UnitTable u = String -> Maybe u. This would allow my library to represent units as strings, and not determine the list of units in advance of invoking the parser (but obviously there would be no ambiguity check in such a case). The existing interface could be supported except that SymbolTable would lose its Show instance. Does this sound reasonable?

goldfirere commented 9 years ago

Yes, this seems like a good idea.

Your comment about the lack of ambiguity check: that would just be in your implementation, right? Because it seems I can still do an ambiguity check in my implementation.

No problems about the loss of a Show instance.

But, here's the biggest question of all: What should the new module be named??

goldfirere commented 9 years ago

I've moved the TH definitions from Internal to the main Parser module. The only reason they were in Internal was to make it easier to test the parser. But, I just put them in Parser and said {-# OPTIONS_HADDOCK prune #-} to hide for-internal-use-only (but not unsafe) definitions.

When you do this, please include the parser tests in the new package. Thanks!

adamgundry commented 9 years ago

Great, thanks! I'll finish putting something together next week.

Your comment about the lack of ambiguity check: that would just be in your implementation, right? Because it seems I can still do an ambiguity check in my implementation.

Yes, the existing ambiguity check would work unchanged in the SymbolTable smart constructor. I'll just add another smart constructor for SymbolTable that doesn't have an ambiguity check, but allows the units mapping to be infinite.

But, here's the biggest question of all: What should the new module be named??

Oh no, the second hardest problem in computer science... I've tentatively gone for Data.Metrology.ParseUnit but feel free to choose a different colour for that bikeshed. ;-)

goldfirere commented 9 years ago

Well, my alternative was Text.Parse.Units, because the parser really only deals with units, not dimensions, quantities, and the rest of metrology. In any case, full steam ahead.