fix: permit SGR colour in menu display text - #44
Merged
Conversation
0.16.3 removed a colour carve-out from the kindmap parser, on the grounds that SPEC.md forbade control characters in a display and RFC 1436 asks that the field hold only printable characters "since many different clients will be using it". The reasoning was sound and the conclusion was wrong, because it argued from the 1993 text rather than from gopherspace. gopher://baud.baby/1/ builds its root menu out of truecolour SGR info lines. A bridge on the strict rule renders them as literal escape text, so 0.16.3 regressed proxying against a real, working hole: before i<ESC>[38;2;27;75;105m⣶ ... after i [38;2;27;75;105m⣶ ... The rule is now: a display may carry SGR and nothing else addressable, a link carries no control character at all. A link is parsed and acted upon; a display is only shown. That keeps the property worth keeping and stops the bridge mangling gopherspace. cleanTerminalDisplay deliberately does not use stripVTControlCharacters: removing a whole sequence and replacing each control character with a space give different strings, and the specification says spaces. A second implementation reading the prose writes the simple version, so this has to be the simple version too. That mismatch was live for a few minutes here and is the same class of bug as the one being fixed. Fixture v3 pins what survives and what is spaced out. The Python client was updated from SPEC.md prose and agrees.
Merged
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
1 participant
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.
0.16.3 removed the colour carve-out from the kindmap parser. I argued it from
RFC 1436's advice that the display field hold only printable characters "since
many different clients will be using it". That reasoning was sound and the
conclusion was wrong: it argued from the 1993 text rather than from gopherspace
as it actually is.
gopher://baud.baby/1/builds its root menu out of truecolour SGR info lines:A bridge on the strict rule turns that into literal escape text, so 0.16.3
regressed proxying against a real, working hole:
The rule now: a display may carry SGR and nothing else addressable; a link
carries no control character at all. A link is parsed and acted upon, a display
is only shown. That keeps the property worth keeping and stops the bridge
mangling gopherspace.
One subtlety worth calling out, because it is the same class of bug as the one
being fixed.
cleanTerminalDisplaydoes not usestripVTControlCharacters:removing a complete sequence and replacing each control character with a space
produce different strings, and the spec says spaces. The first version of this
change used the strip and diverged from the Python client within minutes; the
fixture caught it.
SPEC.md: the rule, with the reasoning for why SGR specifically.agrees. Its independence is weaker than for v1 and v2 since the same author
wrote both by now;
docs/nip-submission.mdsays so plainly rather thanoverclaiming.
a background on every cell and painted a near-black slab across a light
terminal.
tools/halfblock.mjsgained--ink=R,G,B, and its decode checkcaught a comma-for-semicolon bug in the sequence on the first run.
324 tests,
npm run checkgreen, Python 5/5 against the same bytes(sha256
d461eedf…).