Repository navigation
feat(units): ten dimensions, and a split that resolves instead of guessing - #17
Merged
Merged
Conversation
… guessing Unit conversion knew length, mass and temperature, so `2 h in min`, `10 GB in MB`, `5 km/h in mph`, `3 tbsp in ml` and `1 bar in psi` all fell through the cascade to a web search. The table now carries a dimension with each factor — time, volume, area, speed, data, pressure and energy join the original three — and conversion requires the two units to share one. The amount side is parsed by resolution, not by cutting at a fixed column: - candidate connector splits are tried right to left, and the first one that resolves on both sides wins, so `10 in in cm` is ten inches into centimetres rather than a failed split on the first " in "; - a suffix is only accepted as a unit when a number (or an expression the calc engine already handles) sits in front of it, so `1e3 m in km` and `2*3 km in mi` work and `5min` is still minutes, not 5 metres + "in". Factors follow Numbat's unit definitions (MIT/Apache-2.0); only the physical constants are reused, credited in the source. No dependency added.
`/cmd` cuts an output line at 500 characters and appended a bare ellipsis, so a wrapped line and a 40 KB minified JSON line looked the same. The marker now carries the count — `… (+39412 chars)` — which makes the size of what is missing visible without printing any of it.
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.
What I read
CLDR's unit conversion lived in one function,
extras.rs::convert_units, over three tables — length (9), mass (7), temperature (3) — reached from the smart cascade after math and before path / PATH-binary / web. The amount side was split by a byte scan that only accepted[-+]?digits[.digits].What was wrong
Everything outside those three dimensions fell silently through to a web search. Measured against the old logic:
2 h in min,90 min in h,10 GB in MB,5 km/h in mph,3 tbsp in ml,1 bar in psi→None, i.e. a DuckDuckGo query. Two parse bugs in the same function:10 in in cm(the inch collides with the" in "connector) and1e3 m in km(no exponent in the literal).What I changed
Units (minor). One table carries a dimension with every factor; conversion requires both units to share a dimension. Dimensions: length, mass, time, volume, area, speed, data, pressure, energy, temperature. Data is decimal (
kb= 1000 B) with the binary forms spelled out (kib,mib, …). Factors follow Numbat's unit definitions (MIT/Apache-2.0) — physical constants only, no code, credited in the source. No dependency added.Parsing. Candidate connector splits are tried right to left and the first that resolves on both sides wins, so
10 in in cmis ten inches into centimetres. A suffix is only taken as a unit when a number — or an expression the existing calc engine already handles — sits in front of it, so1e3 m in kmand2*3 km in miwork while5minstays minutes rather than 5 metres + "in"./cmd truncation (patch). A line cut at 500 characters now ends
… (+N chars)instead of a bare ellipsis, so a wrapped line and a 40 KB minified JSON line no longer look the same.Docs follow the behaviour:
docs/USER_GUIDE.md,docs/index.html, the README feature row and the/cmdnote. Twochanges/files carry the bump levels;VERSIONis untouched.Evidence
CI on this branch —
gh run view 42613276705: bothCheckjobs green (ubuntu-latest 26s, windows-latest 1m17s).Five new tests (
conversion_time_volume_data,conversion_speed_area_pressure_energy,conversion_parsing_resolves_the_split,long_lines_say_how_much_was_cut) plus four new--self-testchecks (14 → 18). No benchmark is claimed.What I deliberately did not do
10 km in mi → 6.213712 mi(with the unit); the code has never appended it. Pre-existing doc/behaviour mismatch, left alone rather than folded into this diff.km/h,m/s,mphare table entries;5 ft 2 in in cm(two amount tokens) still does not parse.cargo fmt. The rustfmt step was already red onmain(50 diffs across 7 files, all line width) and is non-blocking; my additions follow the file's existing long-line style and add 9 diffs of the same kind. Cleaning that is a separate sweep, not this PR.