improvement!: replace Localize.Unit with a generated, dependency-free BB.Unit - #227
Draft
jimsynz wants to merge 1 commit into
Draft
improvement!: replace Localize.Unit with a generated, dependency-free BB.Unit#227jimsynz wants to merge 1 commit into
Localize.Unit with a generated, dependency-free BB.Unit#227jimsynz wants to merge 1 commit into
Conversation
…ee `BB.Unit`
Localize is ~231 modules plus 4.1MB of CLDR locale data, all of which had to
compile and ship for a robot that only ever converts metres, radians and
newton-metres. On a Nerves target that costs more than the project itself.
`mix localize.unit.gen_conversions` emits a self-contained conversion module:
CLDR's conversion tables plus Localize's own unit parser, expanded by
`mix nimble_parsec.compile` so the grammar is shared rather than
reimplemented. The result is checked in as `lib/bb/unit/conversions.ex` and
Localize becomes a `:dev`-only dependency, pinned to a git ref because the
task is not yet released.
`%Localize.Unit{}` is replaced throughout by `%BB.Unit{name:, value:}`, which
carries the same two fields. `compare/2`, `compatible?/2` and `convert/2` are
derived from the generated `resolve/1`, and agree with Localize on every unit
the DSL uses. `to_string!/2` loses its `:style` option — its five call sites
are all error messages — and an unknown or incompatible unit now raises
`BB.Error.Invalid.Unit` rather than a Localize exception.
The generated file is excluded from credo, whose underscore-in-large-numbers
check objects to CLDR constants such as `149597870700.0`, and carries one
dialyzer exemption for an unreachable clause in the generator's own runtime
body. Both are the generator's to fix; neither is editable here, since any
change is lost on regeneration.
The prod dependency tree loses localize, decimal, gettext, nimble_parsec and
expo; a target carries 3 modules and 177KB in their place.
BREAKING CHANGE: DSL unit values are `%BB.Unit{}` rather than `%Localize.Unit{}`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Spike: drop Localize from the runtime and ship a generated conversion module instead.
Not for merge yet — the generator is unreleased, so
:localizeis pinned to a git ref. Opening this to look at the shape and to give Kip a real consumer ofmix localize.unit.gen_conversions.Why
Localize is ~231 modules and 4.1 MB of CLDR locale data, plus
decimal,gettext,nimble_parsecand an optionalc_srcNIF. All of it compiles and ships so that a robot can convert metres, radians and newton-metres. On a Nerves target that is more build cost than the project itself.What the generator does
mix localize.unit.gen_conversions(elixir-localize/localize@b77cb38c, one commit pastv1.0.1) emits a self-contained module. It takes no unit selection at all: rather than tabulating a chosen set, it inlines Localize's own unit parser — already written as a NimbleParsec template — expanded viamix nimble_parsec.compile, with theLocalize.*references rewritten out. Parity withLocalize.Unitis structural rather than maintained: there is no second grammar to keep in step.Checked in here as
lib/bb/unit/conversions.ex(4,670 lines, 154 KB, 3 modules). Regeneration is byte-identical across runs — the task strips its own path/timestamp banner deliberately.Verification
All 23 distinct units the ecosystem uses resolve to the same base unit and factor as Localize itself, including the compounds that no
--types-style selection could have reached:newton-meterkilogram-square-meter-per-square-secondkilogram-square-meterkilogram-square-meternewton-second-per-meterkilogram-per-secondnewton-meter-second-per-{degree,radian}kilogram-square-meter-per-second-revolution{degree,radian}-per-square-secondrevolution-per-square-secondCross-unit conversion was checked against Localize on 13 pairs (including
newton-meter-second-per-degree → newton-meter-second-per-radian= 57.2957…, andmeter → secondwhich both refuse). Edge cases route correctly too:"bogus","","per-", a 5,000-character input (byte cap),"foot-and-inch"and"beaufort"all return{:error, {:unknown_unit, _}}.mix check --no-retryis green: 1327 tests, credo --strict, dialyzer, reuse, formatter, ex_doc, spark cheat sheets. The test suite needed no changes beyond renaming the struct — no numerical drift.API change
%Localize.Unit{}becomes%BB.Unit{name:, value:}, same two fields.compare/2,compatible?/2andconvert/2are derived from the generatedresolve/1. Unit errors are nowBB.Error.Invalid.Unit.to_string!/2loses its:styleoption — localised rendering is the one thing genuinely not reconstructible. All five call sites were error messages, which now read1.5 meter-per-secondinstead of1.5 m/s.Satellite impact is two lines:
bb_servo_feetechandbb_servo_pigpioeach callLocalize.Unit.convert!once.Cost
The prod dependency tree loses localize, decimal, gettext, nimble_parsec and expo. A target carries 3 modules / 177 KB in their place.
Against that, 154 KB of generated code is now committed and shows up in every CLDR-bump diff, and it needed two tooling exemptions:
149597870700.0tripping the underscore check. New.credo.exsexcludes the file.pattern_match_covon an unreachable catch-all inparse_identifier/1. It is in the generator's hand-written runtime body, not the NimbleParsec expansion. Cosmetic (unknown identifiers are rejected during decomposition, not by the parser), but it fails a--warnings-as-errorsgate. Parked in.dialyzer_ignore.exs.The formatter needed nothing — the generator emits formatted code.
For Kip
Three things worth fixing upstream, none blocking:
reuse lintfails until the consumer adds a.licensesidecar. A--headeroption, or just emitting the consumer's licence, would help.Parser.parse/1keeps@spec ... :: {:error, Exception.t()}afterlocalize_free/2rewrites the constructor to return a plain{:parse_error, bindings}tuple — likely the cause of (1).Open questions
BB.Parameter.Store.Detsserialises unit values; anything already on disk holds%Localize.Unit{}and will not load into%BB.Unit{}. Needs a migration or a documented reset.