Skip to content

Fix MacOSAppVersion ordering - #80

Merged
psobot merged 1 commit into
masterfrom
psobot/fix-version-comparison
Aug 8, 2026
Merged

psobot merged 1 commit into
masterfrom
psobot/fix-version-comparison

Conversation

@psobot

@psobot psobot commented Aug 8, 2026 •

Copy link
Copy Markdown
Owner

Via my friend Claude:

Prerequisite for any multi-version work, but a real bug on its own.

The defect

__lt__ and __le__ OR'd three independent comparisons together instead of comparing lexicographically, so a version could sort below another in both directions at once. Not hypothetical — this is the exact pair in play:

14.4 < 14.5 -> True
14.5 < 14.4 -> True     # because "1A107s" < "1A89s" as plain strings

LATEST_VERSION is max(VERSIONS), and VERSIONS is built by iterating os.listdir(). So the moment a second version is bundled, which version counts as "latest" depends on directory order. With one version bundled this was invisible, which is why it survived.

It also affects the user-facing "your Keynote is newer than this parser supports" warning, which compares __supported_keynote_version__ < installed_version.

Three defects, one fix

Before Now
Antisymmetry a < b and b < a both true single tuple comparison
Digit runs "1A107s" < "1A89s", "7043.0.93" < "7043.0.9" split on digit runs, compared numerically
Differing lengths short_version_comparator scaled by 10 ** (len - i), so 14.4.1 scored 14410 vs 14.5 at 1450 — older sorting higher plain tuple comparison

Also adds __eq__ and __hash__ — previously two identical versions compared unequal, since equality fell back to identity — and derives the rest with functools.total_ordering.

Removal

short_version_comparator is gone. It was internal (the only references were inside this module) and its value wasn't meaningful across versions with differing component counts. Flagging it in case you consider it public API.

Verification

The new tests fail 6 of 9 against the previous implementation and pass against this one. Full suite 39 passed; lint and format clean.

__lt__ and __le__ OR'd three independent comparisons together rather than
comparing lexicographically, so a version could sort below another in both
directions at once. That is not hypothetical for the versions in play:

    14.4 < 14.5 -> True
    14.5 < 14.4 -> True     # because "1A107s" < "1A89s" as plain strings

LATEST_VERSION is max(VERSIONS), and VERSIONS is built by iterating
os.listdir(), so with two versions bundled the "latest" version would have
depended on directory order. With one version bundled this was invisible,
which is why it survived.

Three separate defects, all fixed by comparing a single tuple:

 - Antisymmetry, as above.
 - Digit runs inside version strings compared as text, so "1A107s" sorted
   before "1A89s" and "7043.0.9" after "7043.0.93". Now split on digit runs
   and compared numerically.
 - short_version_comparator scaled each component by 10 ** (len - i), so the
   score depended on how many components a version had: "14.4.1" scored 14410
   against "14.5" at 1450, making the older release sort higher. Replaced with
   a plain tuple comparison, which handles differing lengths correctly.

Adds __eq__ and __hash__ (previously two equal versions compared unequal,
since equality fell back to identity) and derives the rest via total_ordering.

Removes short_version_comparator. It was internal - the only references were
inside this module - and its value was not meaningful across versions with
differing component counts.

Verified the new tests fail on the previous implementation: 6 of 9.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179M4xvAKPGrgKpy4AeCsM7
@psobot
psobot merged commit a82103d into master Aug 8, 2026
4 checks passed
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

Successfully merging this pull request may close these issues.

1 participant