diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index fe610daa..448639d7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -74,10 +74,17 @@ jobs: name: dist path: dist/ + # skip-existing makes a re-run idempotent. Without it, a release that got + # its files onto PyPI some other way -- a manual twine upload, or a first + # attempt that uploaded and then failed later in the job -- can never be + # re-run, because the upload aborts on "file already exists". That took + # 1.0.1, 1.1.0 and 1.2.0 with it: the publish step failed, the release + # job below never ran, and the repository has tags with no releases. - name: Publish to PyPI uses: pypa/gh-action-pypi-publish@release/v1 with: packages-dir: dist/ + skip-existing: true release: name: Create GitHub Release diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a3c0869..ad2584ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,58 @@ Notable changes, newest first. Dates are ISO 8601. +## Unreleased + +### Header typos were swallowed in silence + +`tempo: banana` compiled happily at 100. `time: 3-4` compiled at 4/4. `key: Zz` +sounded in C while a chart printed `Zz` over it. None of them said anything — +not on `compile`, not on `check`, and not on `info --verbose`, which documents +itself as showing every diagnostic. + +Each still falls back to a default, which is right: a typo in a header should +not cost you the piece. What was wrong is falling back **without saying so**, +the same fault as `Xm9` compiling to a silent bar. Now: + +``` +warning: tempo 'banana' is not a number; using 100 +warning: time '3-4' is not a metre; using 4/4 +warning: key 'Zz' is not a key; sounding in C major +``` + +The key message names the *sounding* key rather than the text, because the +unreadable text is kept for display — so "using Zz major" would have been no +help at all. + +The parsers do not raise on nonsense, so these could not simply catch an +exception. `theory.KEY_RE` and `Meter.readable` name each rule once and serve +both the parse and the question "will parsing work", rather than a second copy +of each pattern drifting from the first. + +Found by a fresh agent installing from PyPI and typing plausible mistakes — a +test this project cannot run on itself, having long since stopped making +beginner errors. It immediately turned up a real one in the bundled songbook: +`time: 2/4 (Lassan) then 4/4 (Friska)` on the Hungarian Rhapsody, a human +annotation the metre field cannot express, silently becoming 4/4. The file is +left alone — changing the metre would change the music — and the warning now +says what was ignored. + +### The README is the PyPI page, and its links did not work there + +`pyproject.toml` sets `readme = "README.md"`, so that file *is* the project page. +Forty-four relative links and one relative image resolved on GitHub and 404'd on +PyPI, which is where a `pip install` user reads them. The page told people to +read a dozen documents none of which they could reach. + +All absolute now, images through `raw.githubusercontent.com` because a `blob` +URL serves HTML rather than an image. `tests/test_readme_links.py` buys back the +guarantee absolute links lose: every URL into this repository is mapped to a path +and checked to exist, images must not use `blob`, and the version the README +claims must match the tree — it said "Version 1.0" while PyPI shipped 1.2.0. + +The examples section also told pip users to compile `examples/…`, which the wheel +does not ship. It now says so, and points at the bundled library, which it does. + ## 1.2.0 — 2026-08-18 A minor rather than a patch: `notation/merge.py`, `notation/lyrics.py` and diff --git a/CLAUDE.md b/CLAUDE.md index 9898e3ca..5c612491 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -296,13 +296,25 @@ famous for came from those rows and are now 2. - The built-in synthesiser is a preview renderer; timbres are approximations. Audio is mono. - The host bridge cannot stream and reports no token usage. -- **`plainsong/mcp/` also exists in `SuperInstance/plainsong-mcp`.** This is the - one open violation of "one of everything" and it is deliberate but temporary: - the extraction happened while this branch was in review. The intended end - state is that the MCP server lives only in the sibling repository and this - package stops carrying it. Until that lands, a change to one copy must be - made to the other or they will drift — which is precisely the failure mode - the rule exists to prevent. Do not build anything new on this copy. +- **`plainsong/mcp/` also exists in `SuperInstance/plainsong-mcp`.** The one + open violation of "one of everything". Do not build anything new on this copy. + + **This has now cost something real, so it is no longer a theoretical rule.** + The two copies were measured: 240 lines of difference across seven of eight + files. Most is mechanical — relative versus absolute imports, unavoidable when + the same code lives inside and outside a package. One was not. The HTTP + transport's DNS-rebinding guard (`_host_is_local`, requiring a loopback + `Host` because Origin-against-Host alone is defeated by rebinding) existed + **here and not in the sibling**, which is the copy people `pip install` for + MCP. Fixed there, with tests, but it sat open for months with nothing in + either repository able to notice. + + The injection machinery in `mcp/tools.py` and `mcp/resources.py` — the + `ensemble=` parameter and `_default_ensemble()` — is **unused**. It was + written so the sibling could import this `tools.py` and pass its own ensemble; + the sibling never does, having its own copy bound by relative import. Do not + extend it: the end state is that MCP lives only in the sibling and this + directory goes. - The MCP server has now been driven by a third-party client: the official `mcp` Python SDK 2.0.0, over stdio. `initialize`, `tools/list`, `resources/list`, `prompts/list`, `tools/call` and `resources/read` all diff --git a/README.md b/README.md index 8f35ad0f..9aae4b1e 100644 --- a/README.md +++ b/README.md @@ -16,14 +16,14 @@ file any editor, any diff tool and any language model can read. **If you read music but not code**, you already understand most of that block — it is a lead sheet with the bars drawn in. Start with -[Your first song](docs/tutorial-first-song.md); it assumes nothing about +[Your first song](https://github.com/SuperInstance/plainsong/blob/master/docs/tutorial-first-song.md); it assumes nothing about programming and gets you to something you can hear. **If you write code but not music**, the thing to know is that you never declare durations. You write how many events happen in a bar and the bar divides -itself. Start with [the notation reference](docs/notation.md). +itself. Start with [the notation reference](https://github.com/SuperInstance/plainsong/blob/master/docs/notation.md). -**If you are an agent**, read [AGENTS.md](AGENTS.md) first. It is short, and +**If you are an agent**, read [AGENTS.md](https://github.com/SuperInstance/plainsong/blob/master/AGENTS.md) first. It is short, and most of it is the mistakes other agents have already made here. ## Try it without installing anything @@ -148,7 +148,7 @@ plainsong lyrics song.song # which note each syllable really lands on Set `core.lyrics = "bound"` and each syllable is sung on the note it is written under, with the barline resyncing so a miscount costs one bar instead of the rest of the song. The default leaves existing files alone. See -[docs/lyrics.md](docs/lyrics.md). +[docs/lyrics.md](https://github.com/SuperInstance/plainsong/blob/master/docs/lyrics.md). ### Two rows of the same kind run in sequence @@ -246,7 +246,7 @@ against the standard library. **Audio.** The built-in synthesiser works with nothing installed. For higher quality, install fluidsynth and a General MIDI soundfont — Plainsong will use -them automatically. See [docs/audio.md](docs/audio.md) for installation and +them automatically. See [docs/audio.md](https://github.com/SuperInstance/plainsong/blob/master/docs/audio.md) for installation and configuration. Optional extras — NumPy for faster synthesis, ffmpeg for format conversion, mido @@ -258,7 +258,7 @@ for hardware MIDI — are detected when present and never required. plainsong chart song.song -o chart.svg ``` -![a chord chart rendered from one of the bundled examples](docs/img/creatures-of-interval.svg) +![a chord chart rendered from one of the bundled examples](https://raw.githubusercontent.com/SuperInstance/plainsong/master/docs/img/creatures-of-interval.svg) That image is the SVG above, committed to this repository and embedded with an `` tag — which is the only way a chart appears in markdown on a platform @@ -270,7 +270,7 @@ fonts it names. Bars are as wide as their contents need, worked out from the same measurements. It reads `prefers-color-scheme`, so it is legible in a dark README as well as a light one. -See [docs/chart.md](docs/chart.md), including what it deliberately is not: a +See [docs/chart.md](https://github.com/SuperInstance/plainsong/blob/master/docs/chart.md), including what it deliberately is not: a chord chart rather than an engraver, with no noteheads, staff or beaming. ## Timing that models the room @@ -300,7 +300,7 @@ what conductor hears, against the written beat The organist's key goes down 241 ms early so the pipe speaks on the beat. At the podium everything lands together; at any player's desk it does not, which is why an orchestra watches rather than listens. See -[docs/performance.md](docs/performance.md). +[docs/performance.md](https://github.com/SuperInstance/plainsong/blob/master/docs/performance.md). Without a `[Stage]` block none of this applies and written times are taken at face value, exactly as before. @@ -321,7 +321,7 @@ Groq, Mistral, Together, Fireworks, Cerebras, Azure), with local servers (Ollama LM Studio, vLLM, llama.cpp), and with **no key at all** when you are already running inside another agent such as Claude Code — it borrows the model that is already there. Providers are catalogue entries rather than code, so adding one is -a JSON file. See [docs/providers.md](docs/providers.md). +a JSON file. See [docs/providers.md](https://github.com/SuperInstance/plainsong/blob/master/docs/providers.md). Try it with no key and no network at all: @@ -380,21 +380,33 @@ Every command takes `--json`. Use it when parsing output. Eight complete pieces ship in the repository, written to be read rather than generated. Each one is a template: copy it, change the chords, recompile. +These live in the repository rather than the wheel, so a `pip install` does not +put them on your disk. Clone, or read them at the links below: + +```bash +git clone https://github.com/SuperInstance/plainsong +plainsong compile plainsong/examples/plainsong-1-creatures-of-interval.song --audio out.wav +``` + +With only `pip install`, the bundled library is the equivalent — several thousand +charts that *are* shipped: + ```bash -plainsong compile examples/plainsong-1-creatures-of-interval.song --audio out.wav +plainsong library "waltz" # search what came with the package +plainsong play stand-by-me # render and play one by name ``` | | | |---|---| -| [Creatures of Interval](examples/plainsong-1-creatures-of-interval.song) | Four voices, plain 4/4 — the simplest complete piece | -| [The Room Is Safe](examples/plainsong-2-the-room-is-safe.song) | Lyrics and melody together | -| [Hermes Blues](examples/plainsong-3-hermes-blues.song) | Twelve-bar blues with swing | -| [Closing Time](examples/plainsong-4-closing-time.song) | The relative dialect — roman numerals and scale degrees | -| [Deck Work](examples/plainsong-5-deck-work.song) | Several named players | -| [Spacing: melody](examples/plainsong-6-spacing-melody.song) · [chords](examples/plainsong-7-spacing-chords.song) · [dashes](examples/plainsong-8-spacing-dashes.song) | How duration-by-spacing behaves | +| [Creatures of Interval](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-1-creatures-of-interval.song) | Four voices, plain 4/4 — the simplest complete piece | +| [The Room Is Safe](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-2-the-room-is-safe.song) | Lyrics and melody together | +| [Hermes Blues](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-3-hermes-blues.song) | Twelve-bar blues with swing | +| [Closing Time](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-4-closing-time.song) | The relative dialect — roman numerals and scale degrees | +| [Deck Work](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-5-deck-work.song) | Several named players | +| [Spacing: melody](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-6-spacing-melody.song) · [chords](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-7-spacing-chords.song) · [dashes](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-8-spacing-dashes.song) | How duration-by-spacing behaves | And five deliberately awkward files in -[`examples/edge-cases/`](examples/edge-cases/) — polyrhythm, tempo shifts, dense +[`examples/edge-cases/`](https://github.com/SuperInstance/plainsong/blob/master/examples/edge-cases/) — polyrhythm, tempo shifts, dense chords, players with no chord row, and a kitchen sink. They exist to be compiled, not imitated. @@ -406,36 +418,36 @@ plainsong library --collections # what is in there plainsong play stand-by-me # render and play by name ``` -See [the songbook](docs/songbook.md) for what is in it and why it is chord +See [the songbook](https://github.com/SuperInstance/plainsong/blob/master/docs/songbook.md) for what is in it and why it is chord charts only. ## Documentation | | | |---|---| -| [For agents](AGENTS.md) | **Read this first if you are one.** The contract, and the mistakes agents make here | -| [Your first song](docs/tutorial-first-song.md) | Thirty minutes from install to a piece you wrote | -| [Arranging](docs/tutorial-arranging.md) | Several players, time signatures, and the stage model | -| [Integration](docs/integration.md) | Driving Plainsong from other software | -| [Getting started](docs/getting-started.md) | From clone to a finished piece | -| [Notation reference](docs/notation.md) | The whole language | -| [Performance timing](docs/performance.md) | Stages, arrival times, conductor directives | -| [Audio](docs/audio.md) | Rendering to WAV, MP3, and other formats; fluidsynth with soundfonts | -| [MCP server](docs/mcp.md) | Driving the system from any MCP client | -| [Ensemble sessions](docs/ensemble.md) | Several agents co-authoring one score | -| [Providers](docs/providers.md) | Connecting a model, adding your own | -| [Host bridge](docs/host-bridge.md) | Running under another agent, with no key | -| [Agents](docs/agents.md) | The composer and build agents, and their tools | -| [Chords](docs/chords.md) | Which symbols are understood, and the rules that derive the notes | -| [Voicing](docs/voicing.md) | Which notes sound when a chord names more than fit, and how that was measured | -| [Lyrics](docs/lyrics.md) | Binding syllables to the notes they are sung on, and why padding is not melisma | -| [Charts](docs/chart.md) | Drawing a chord chart as SVG, and the measurements behind it | -| [The songbook](docs/songbook.md) | The bundled charts, and the copyright policy they follow | -| [Connectors](docs/connectors.md) | Getting notation and audio into other systems | -| [Architecture](docs/architecture.md) | How it fits together, and why | -| [Specs](docs/specs.md) | The checks the system runs against itself | -| [Releasing](docs/releasing.md) | How a version is cut | -| [Contributing](CONTRIBUTING.md) | Getting involved | +| [For agents](https://github.com/SuperInstance/plainsong/blob/master/AGENTS.md) | **Read this first if you are one.** The contract, and the mistakes agents make here | +| [Your first song](https://github.com/SuperInstance/plainsong/blob/master/docs/tutorial-first-song.md) | Thirty minutes from install to a piece you wrote | +| [Arranging](https://github.com/SuperInstance/plainsong/blob/master/docs/tutorial-arranging.md) | Several players, time signatures, and the stage model | +| [Integration](https://github.com/SuperInstance/plainsong/blob/master/docs/integration.md) | Driving Plainsong from other software | +| [Getting started](https://github.com/SuperInstance/plainsong/blob/master/docs/getting-started.md) | From clone to a finished piece | +| [Notation reference](https://github.com/SuperInstance/plainsong/blob/master/docs/notation.md) | The whole language | +| [Performance timing](https://github.com/SuperInstance/plainsong/blob/master/docs/performance.md) | Stages, arrival times, conductor directives | +| [Audio](https://github.com/SuperInstance/plainsong/blob/master/docs/audio.md) | Rendering to WAV, MP3, and other formats; fluidsynth with soundfonts | +| [MCP server](https://github.com/SuperInstance/plainsong/blob/master/docs/mcp.md) | Driving the system from any MCP client | +| [Ensemble sessions](https://github.com/SuperInstance/plainsong/blob/master/docs/ensemble.md) | Several agents co-authoring one score | +| [Providers](https://github.com/SuperInstance/plainsong/blob/master/docs/providers.md) | Connecting a model, adding your own | +| [Host bridge](https://github.com/SuperInstance/plainsong/blob/master/docs/host-bridge.md) | Running under another agent, with no key | +| [Agents](https://github.com/SuperInstance/plainsong/blob/master/docs/agents.md) | The composer and build agents, and their tools | +| [Chords](https://github.com/SuperInstance/plainsong/blob/master/docs/chords.md) | Which symbols are understood, and the rules that derive the notes | +| [Voicing](https://github.com/SuperInstance/plainsong/blob/master/docs/voicing.md) | Which notes sound when a chord names more than fit, and how that was measured | +| [Lyrics](https://github.com/SuperInstance/plainsong/blob/master/docs/lyrics.md) | Binding syllables to the notes they are sung on, and why padding is not melisma | +| [Charts](https://github.com/SuperInstance/plainsong/blob/master/docs/chart.md) | Drawing a chord chart as SVG, and the measurements behind it | +| [The songbook](https://github.com/SuperInstance/plainsong/blob/master/docs/songbook.md) | The bundled charts, and the copyright policy they follow | +| [Connectors](https://github.com/SuperInstance/plainsong/blob/master/docs/connectors.md) | Getting notation and audio into other systems | +| [Architecture](https://github.com/SuperInstance/plainsong/blob/master/docs/architecture.md) | How it fits together, and why | +| [Specs](https://github.com/SuperInstance/plainsong/blob/master/docs/specs.md) | The checks the system runs against itself | +| [Releasing](https://github.com/SuperInstance/plainsong/blob/master/docs/releasing.md) | How a version is cut | +| [Contributing](https://github.com/SuperInstance/plainsong/blob/master/CONTRIBUTING.md) | Getting involved | ## Testing @@ -461,7 +473,7 @@ in the documentation that stopped compiling would fail the build. ## Status -Version 1.0. The notation, the CLI surface and the provider catalogue format are +Version 1.2. The notation, the CLI surface and the provider catalogue format are stable; changes to them will go through a deprecation cycle. This release is a rebuild rather than an increment. The previous engine assumed a @@ -481,7 +493,7 @@ Known limits, in the open: - The built-in synthesiser produces mono audio with synthetic timbres. It works immediately with no dependencies. For real instrument samples, install fluidsynth and a soundfont (five minutes on any OS). See - [docs/audio.md](docs/audio.md). + [docs/audio.md](https://github.com/SuperInstance/plainsong/blob/master/docs/audio.md). - The host bridge cannot stream and reports no token usage. - The TUI needs `curses`, which stock Python on Windows does not ship. - No third-party MCP client has connected to the server yet; its protocol @@ -489,9 +501,9 @@ Known limits, in the open: - The bundled songbook (`plainsong/songbook/`) is **chord charts only** — melody and lyric rows were removed from all 6,309 files. A chord progression is not protectable expression; a tune and its words are, and nothing in those generated files recorded provenance. - See [docs/songbook.md](docs/songbook.md). + See [docs/songbook.md](https://github.com/SuperInstance/plainsong/blob/master/docs/songbook.md). - The two earlier engines are in `legacy/`, unmaintained. Nothing imports them. ## Licence -MIT. See [LICENSE](LICENSE). +MIT. See [LICENSE](https://github.com/SuperInstance/plainsong/blob/master/LICENSE). diff --git a/docs/releasing.md b/docs/releasing.md index d5b52006..9a39c5ec 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -18,11 +18,32 @@ python3 -m unittest discover -s tests python3 -m plainsong spec python3 -m plainsong check docs examples plainsong/songbook README.md -# 4. Tag and push. +# 4. Tag the commit that carries the bump -- not whatever master happened to be +# when you started. Merge the version bump FIRST, then pull, then tag. +git pull +grep __version__ plainsong/version.py # must be the version you are tagging git tag -a v1.1.0 -m "1.1.0" git push origin v1.1.0 ``` +**Tag the right commit.** `v1.2.0` was once created on the commit before the +version bump merged. The tree there still said 1.1.0, so the workflow's tag +check refused it -- correctly, and that is the guard doing its job -- and a +local `python -m build` in the same clone quietly produced `1.1.0` artifacts +that PyPI then rejected as duplicates. The one-line `grep` above is what catches +it. If you have already pushed a tag to the wrong commit: + +```bash +git tag -d v1.2.0 +git push origin :refs/tags/v1.2.0 # delete it from the remote +git pull # get the real bump +git tag -a v1.2.0 -m "1.2.0" +git push origin v1.2.0 +``` + +Do not use a shallow (`--depth 1`) clone for that: deleting and re-pushing a tag +needs the real history. + ## What the workflow then does `.github/workflows/release.yml` triggers on any tag matching `v*`: @@ -39,6 +60,13 @@ git push origin v1.1.0 [Trusted Publishing](https://docs.pypi.org/trusted-publishers/). There is no API token in the repository and there should never be one: the job authenticates with a short-lived OIDC identity, which is why it declares `id-token: write`. + It passes `skip-existing`, so a version already on PyPI is skipped rather than + failing the job. That matters more than it sounds: without it, a release whose + files reached PyPI some other way -- a manual `twine upload`, or an attempt + that uploaded and then failed later -- can never be re-run, because the upload + aborts on "file already exists" and the **release** step below never gets to + run. That is exactly how 1.0.1, 1.1.0 and 1.2.0 ended up as tags with no + GitHub Release. 4. **release** — creates the GitHub Release with the artifacts attached and generated notes. This needs `contents: write`; without it the step fails at the very end, after a successful publish, which is the worst place to find out. diff --git a/plainsong/notation/ir.py b/plainsong/notation/ir.py index e1f3b3f7..5010b975 100644 --- a/plainsong/notation/ir.py +++ b/plainsong/notation/ir.py @@ -138,15 +138,30 @@ def __str__(self) -> str: return f"{self.numerator}/{self.denominator}" @classmethod - def parse(cls, text: str) -> Meter: + def readable(cls, text: str) -> bool: + """Whether `parse` will get a metre out of this rather than defaulting. + + The parser needs to tell "you wrote 4/4" from "you wrote nonsense and + got 4/4", and it cannot do that from the result alone. + """ + try: + numerator, slash, denominator = text.strip().partition("/") + except AttributeError: + return False + if not slash and not numerator.strip().isdigit(): + return False try: - numerator, _, denominator = text.strip().partition("/") meter = cls(int(numerator), int(denominator or 4)) - if meter.numerator < 1 or meter.denominator not in (1, 2, 4, 8, 16, 32): - return cls() - return meter except (ValueError, AttributeError): + return False + return meter.numerator >= 1 and meter.denominator in (1, 2, 4, 8, 16, 32) + + @classmethod + def parse(cls, text: str) -> Meter: + if not cls.readable(text): return cls() + numerator, _, denominator = text.strip().partition("/") + return cls(int(numerator), int(denominator or 4)) @dataclass diff --git a/plainsong/notation/parser.py b/plainsong/notation/parser.py index 8b809cb1..9ca46f85 100644 --- a/plainsong/notation/parser.py +++ b/plainsong/notation/parser.py @@ -346,27 +346,71 @@ def _handle_metadata(self, line: str, index: int) -> None: if not value: continue try: - self._apply_metadata(key, value) + self._apply_metadata(key, value, index) except Exception as exc: # never let a header field abort a parse self._note("warning", f"could not read {key} = {value!r} ({exc})", index) - def _apply_metadata(self, key: str, value: str) -> None: + def _apply_metadata(self, key: str, value: str, index: int = 0) -> None: + """Read one header field, and say so when it cannot be read. + + Every one of these falls back to a default rather than failing, which is + right -- a typo in a header should not cost you the piece. What was + wrong is that it fell back in *silence*: `tempo: banna` compiled happily + at 100, `time: 3-4` at 4/4, and nothing anywhere said why the piece came + out at the wrong speed or in the wrong metre. The parsers below do not + raise on nonsense, so the check has to be here. + """ if key == "key": self.meta.key = theory.parse_key(value) + if not theory.KEY_RE.match(value.strip()): + # `parse_key` keeps the text it was given, so an unreadable key + # is displayed on a chart while the music sounds in C. + # Report what will *sound*, not the text that was kept. The + # unreadable text is carried through for display, so a chart + # would print `Zz` over music in C and the warning saying "using + # Zz major" would be no help at all. + sounding = theory.NOTE_NAMES_SHARP[self.meta.key.tonic_pc % 12] + self._note( + "warning", + f"key {value!r} is not a key; sounding in {sounding} {self.meta.key.mode}", + index, + hint="a key looks like C, Am, F#m, Bb, or a mode: `key: D dorian`", + ) elif key in {"tempo", "bpm"}: number = re.search(r"-?\d+(?:\.\d+)?", value) if number: tempo = float(number.group(0)) self.meta.tempo = min(max(tempo, 20.0), 400.0) + else: + self._note( + "warning", + f"tempo {value!r} is not a number; using {self.meta.tempo:g}", + index, + hint="tempo is beats per minute, as in `tempo: 96`", + ) elif key == "swing": number = re.search(r"-?\d+(?:\.\d+)?", value) if number: swing = float(number.group(0)) self.meta.swing = swing / 100.0 if swing > 1 else swing + else: + self._note( + "warning", + f"swing {value!r} is not a number; using {self.meta.swing:g}", + index, + hint="swing is a percentage, as in `swing: 62%`", + ) elif key == "subdivision": self.meta.subdivision = value elif key in {"time", "meter"}: self.meta.meter = Meter.parse(value) + if not Meter.readable(value): + self._note( + "warning", + f"time {value!r} is not a metre; using {self.meta.meter}", + index, + hint="a metre is two numbers over a slash, as in `time: 6/8`", + ) else: self.meta.extra[key] = value diff --git a/plainsong/notation/theory.py b/plainsong/notation/theory.py index 9b7e2b18..2e60a1e0 100644 --- a/plainsong/notation/theory.py +++ b/plainsong/notation/theory.py @@ -451,12 +451,18 @@ def __eq__(self, other: object) -> bool: } +KEY_RE = re.compile(r"^([A-Ga-g])([#b♯♭]*)\s*(.*)$") +"""What a key has to start with. Named because the parser also needs to ask +whether a header value *is* a key, and asking with a second copy of this +pattern is how the two answers drift apart.""" + + def parse_key(text: str) -> Key: """Parse ``Am``, ``A minor``, ``F# dorian`` or ``Bb`` into a :class:`Key`.""" cleaned = text.strip() if not cleaned: return Key(0, "major") - match = re.match(r"^([A-Ga-g])([#b♯♭]*)\s*(.*)$", cleaned) + match = KEY_RE.match(cleaned) if not match: return Key(0, "major", text=cleaned) letter, accidentals, remainder = match.groups() diff --git a/tests/test_readme_links.py b/tests/test_readme_links.py new file mode 100644 index 00000000..8dbbb519 --- /dev/null +++ b/tests/test_readme_links.py @@ -0,0 +1,77 @@ +"""The README is the PyPI project page, so its links have to work from there. + +`pyproject.toml` sets `readme = "README.md"`, which means this file is rendered +verbatim as the long description on PyPI. A relative link like +`[notation](docs/notation.md)` resolves on GitHub and 404s on PyPI, and a +relative image does not render at all — which is how the published page came to +tell people to read a dozen documents none of which they could reach, and to run +example files that a `pip install` does not put on disk. + +So the links are absolute. The cost of that is losing the guarantee that they +point at something real, since nothing local resolves them any more. This test +buys it back: every GitHub URL into this repository is mapped back to a path and +checked to exist. +""" + +from __future__ import annotations + +import re +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +README = ROOT / "README.md" + +BLOB = "https://github.com/SuperInstance/plainsong/blob/master/" +RAW = "https://raw.githubusercontent.com/SuperInstance/plainsong/master/" + +LINK_RE = re.compile(r"!?\[[^\]]*\]\(([^)\s]+)\)") + + +def targets() -> list[str]: + return LINK_RE.findall(README.read_text(encoding="utf-8")) + + +class TestTheReadmeWorksOnPyPI(unittest.TestCase): + def test_no_relative_links_remain(self): + """A relative link is invisible on PyPI, silently.""" + relative = [ + t + for t in targets() + if not t.startswith(("http://", "https://", "#", "mailto:")) + ] + self.assertEqual(relative, [], f"these 404 on the PyPI page: {relative}") + + def test_every_link_into_this_repo_points_at_a_real_file(self): + missing = [] + for target in targets(): + for prefix in (BLOB, RAW): + if target.startswith(prefix): + path = target[len(prefix) :].split("#")[0] + if not (ROOT / path).exists(): + missing.append(target) + self.assertEqual(missing, [], f"link points at a file that is not here: {missing}") + + def test_images_use_raw_rather_than_blob(self): + """A `blob` URL serves an HTML page, so an pointing at one is a + broken image on PyPI and in any other renderer.""" + images = re.findall(r"!\[[^\]]*\]\(([^)\s]+)\)", README.read_text(encoding="utf-8")) + wrong = [i for i in images if i.startswith(BLOB)] + self.assertEqual(wrong, [], f"image must use raw.githubusercontent.com: {wrong}") + + def test_the_version_it_claims_is_the_version_in_the_tree(self): + """The status line said "Version 1.0" while PyPI shipped 1.2.0.""" + from plainsong.version import __version__ + + claimed = re.search(r"(?m)^Version (\d+\.\d+)\.", README.read_text(encoding="utf-8")) + self.assertIsNotNone(claimed, "the README no longer states a version") + major_minor = ".".join(__version__.split(".")[:2]) + self.assertEqual( + claimed.group(1), + major_minor, + f"README says {claimed.group(1)}, the tree is {__version__}", + ) + + +if __name__ == "__main__": + unittest.main()