From 91581eb1953324f559b6708fb2c2ef40c4bb4d8c Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 8 Sep 2026 08:57:59 +0200 Subject: [PATCH 1/4] format: byte counts are grouped in threes A total printed as 2516582400 B. Eleven digits with nothing to hold on to, and this tool prints byte counts everywhere - it is the whole point of it, so the one number a person came for was the hardest to read. It now prints 2 516 582 400 B, in every message that names bytes: the minimum in tfg formats, the summary a run prints, what a preset says its budget is, and what tfg validate reports. A space rather than a comma. A comma is a thousands mark in some countries and a decimal point in others, and this tool is read in both. Machine output is untouched. Nothing in a manifest or under --json goes through here, because a number there is a number rather than a sentence, so no script anybody has written sees any of this. One guard read the old shape and had to be taught the new one, and what it said while it was wrong is worth keeping: it split the line on the first space, read "1 220 B" as one byte, and reported that the tool refuses the minimum it advertises. It was right about what it saw and wrong about what it meant, which is what a parser splitting on the wrong thing always is. Two tools outside this repository read the same line and needed the same lesson. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 14 +++++++ internal/cli/formats.go | 5 ++- internal/cli/generate.go | 14 +++++-- internal/cli/preset.go | 4 +- internal/cli/recipecmd.go | 5 ++- internal/core/humanise.go | 56 ++++++++++++++++++++++++- internal/guard/smallestaccepted_test.go | 10 ++++- 7 files changed, 95 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5844e36..6db6e9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -191,6 +191,20 @@ because it turns other people's test suites red. ### Changed +- **Byte counts are grouped in threes.** A total used to print as + `2516582400 B`. It now prints as `2 516 582 400 B`, in every message that + names a number of bytes - `tfg formats`, the summary a run prints, what a + preset says its budget is, and what `tfg validate` reports. + + Grouped with a space rather than a comma, because a comma is a thousands mark + in some countries and a decimal point in others, and this tool is read in + both. + + Machine output is untouched. `--json` and the manifest carry numbers rather + than sentences, so nothing that parses those sees any of this. If you have a + script reading a byte count out of the human output, it needs to take the + spaces out. + - **Notes are reported once per thing they say, not once per file.** A run of 25 000 one-byte text files used to print 25 001 `note:` lines, every one of them the same sentence about the label not fitting. It now prints one, with diff --git a/internal/cli/formats.go b/internal/cli/formats.go index 6b9d830..466cb9c 100644 --- a/internal/cli/formats.go +++ b/internal/cli/formats.go @@ -7,6 +7,7 @@ import ( "fmt" "io" + "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/format" ) @@ -109,8 +110,8 @@ func entryFor(d format.Descriptor) formatEntry { // list and ignore the argument, ending with 0 - so there was no way to ask what // a format accepts, and the silence looked like an answer. func describeOne(d format.Descriptor, out io.Writer) { - fmt.Fprintf(out, "%s - %s fidelity, %s deterministic, minimum %d B\n", - d.ID, d.Fidelity, d.Determinism, smallestAccepted(d)) + fmt.Fprintf(out, "%s - %s fidelity, %s deterministic, minimum %s\n", + d.ID, d.Fidelity, d.Determinism, core.ExactBytes(smallestAccepted(d))) fmt.Fprintf(out, " extension %s\n", d.Extension) fmt.Fprintf(out, " padding %s\n", d.Padding.Name) fmt.Fprintf(out, " label %s\n", d.Label) diff --git a/internal/cli/generate.go b/internal/cli/generate.go index 4e516b4..45abb57 100644 --- a/internal/cli/generate.go +++ b/internal/cli/generate.go @@ -351,10 +351,16 @@ func sizesFromFlags(g *generateOpts, errOut io.Writer) (sizes []int64, low, high if errors.Is(err, core.ErrBoundaryTooSmall) { // A number somebody typed, so this is USAGE rather than a problem // with a document. The end above it keeps the code it had. + // Ungrouped, unlike every other count this program prints, and the + // exception is deliberate: this echoes back the number somebody + // typed after --boundary. A message that quotes your input and + // respells it on the way is a message you have to translate before + // you can compare it with what you wrote. fmt.Fprintf(errOut, "tfg: --boundary %d B is too small - %s\n", limit, err) return nil, 0, 0, 0, ExitUsage } if err != nil { + // Ungrouped for the same reason as the line above it. fmt.Fprintf(errOut, "tfg: --boundary %d B is too large - %s\n", limit, err) return nil, 0, 0, 0, ExitRecipe } @@ -387,9 +393,9 @@ func produce(ctx context.Context, targets []engine.Target, opt engine.Options, g // Echo the exact byte count. The exact number is the point of this tool, // and it is what any other tool will show when the user goes to check the // file. - fmt.Fprintf(errOut, "%s in %s, %d B total\n", + fmt.Fprintf(errOut, "%s in %s, %s total\n", core.Count(len(planned), "file", "files"), core.Count(len(targets), "target", "targets"), - engine.TotalBytes(planned)) + core.ExactBytes(engine.TotalBytes(planned))) echoBoundaries(targets, planned, errOut) echoManifestReach(planned, errOut) @@ -530,10 +536,10 @@ func echoBoundaries(targets []engine.Target, planned []engine.PlannedFile, errOu if t.BoundaryLimit <= 0 { continue } - fmt.Fprintf(errOut, "boundary %q around %d B:\n", t.ID, t.BoundaryLimit) + fmt.Fprintf(errOut, "boundary %q around %s:\n", t.ID, core.ExactBytes(t.BoundaryLimit)) for _, f := range planned { if f.Target == t { - fmt.Fprintf(errOut, " %-26s %d B\n", f.Name, f.Plan.Bytes) + fmt.Fprintf(errOut, " %-26s %s\n", f.Name, core.ExactBytes(f.Plan.Bytes)) } } diff --git a/internal/cli/preset.go b/internal/cli/preset.go index c98b8ab..09de047 100644 --- a/internal/cli/preset.go +++ b/internal/cli/preset.go @@ -537,9 +537,9 @@ func describePreset(e *preset.Expansion, b budget, out io.Writer) { fmt.Fprintf(out, " --%-12s the global flag, this preset gives it a default\n", name) } - fmt.Fprintf(out, "\nbudget at these values:\n %s, %s, %d B total, format %s\n", + fmt.Fprintf(out, "\nbudget at these values:\n %s, %s, %s total, format %s\n", core.Count(b.Targets, "target", "targets"), core.Count(b.Files, "file", "files"), - b.Bytes, strings.Join(b.Formats, ", ")) + core.ExactBytes(b.Bytes), strings.Join(b.Formats, ", ")) for _, note := range e.Notes() { fmt.Fprintf(out, "\nnote: %s\n", note) } diff --git a/internal/cli/recipecmd.go b/internal/cli/recipecmd.go index aa8ef7d..d3a11ff 100644 --- a/internal/cli/recipecmd.go +++ b/internal/cli/recipecmd.go @@ -94,8 +94,9 @@ func validate(ctx context.Context, args []string, out, errOut io.Writer) int { }, ExitOK) } - fmt.Fprintf(out, "%s is valid: %s, %s, %d B total\n%s\n", - path, core.Count(len(rec.Targets), "target", "targets"), core.Count(len(planned), "file", "files"), engine.TotalBytes(planned), hash) + fmt.Fprintf(out, "%s is valid: %s, %s, %s total\n%s\n", + path, core.Count(len(rec.Targets), "target", "targets"), core.Count(len(planned), "file", "files"), + core.ExactBytes(engine.TotalBytes(planned)), hash) return ExitOK } diff --git a/internal/core/humanise.go b/internal/core/humanise.go index 37ac11c..f185770 100644 --- a/internal/core/humanise.go +++ b/internal/core/humanise.go @@ -3,6 +3,8 @@ package core import ( "fmt" "math" + "strconv" + "strings" "time" ) @@ -20,7 +22,10 @@ import ( func HumanBytes(n int64) string { const unit = 1024 if n < unit { - return fmt.Sprintf("%d B", n) + // Through ExactBytes rather than its own %d, so the two never spell one + // number two ways. Below 1024 there is nothing to group, which is + // exactly why this is easy to get wrong and leave wrong. + return ExactBytes(n) } div, exp := int64(unit), 0 for n/div >= unit && exp < 3 { @@ -30,6 +35,55 @@ func HumanBytes(n int64) string { return fmt.Sprintf("%.1f %cB", float64(n)/float64(div), "KMGT"[exp]) } +// ExactBytes writes a count out in full, grouped in threes, with its unit. +// +// The exact number is the point of this tool and it can never be replaced by a +// rounded one - but eleven digits in a row is a number nobody reads, and both +// surfaces printed it that way. "2516582400 B" was measured on the window's run +// panel and on four lines of the command line, and the owner's report of it was +// that the bytes are welcome and unreadable, which are both true at once. +// +// Grouped with a space rather than a comma, and that is the one choice here +// worth writing down. A comma is the thousands mark in English and the decimal +// mark for most of Europe, so "2,516" is either two and a half thousand or two +// and a half depending on who is reading - and the people who read this run it +// in every country. A space means the same thing everywhere. +// +// Machine output is untouched on purpose. Nothing in a manifest or under --json +// goes through here, because a number there is a number and not a sentence. +func ExactBytes(n int64) string { + return groupedInThrees(strconv.FormatInt(n, 10)) + " B" +} + +// groupedInThrees puts a space every three digits, counting from the right. +// +// Written out rather than reached for in a library because the one in the +// standard library is about money: golang.org/x/text/message formats to a +// LOCALE, and a locale is exactly what this must not have - the window and the +// command line have to say the same thing on a Polish desktop and an American +// one, and docs/UX.md has the surfaces agreeing as a rule rather than a hope. +func groupedInThrees(digits string) string { + sign := "" + if strings.HasPrefix(digits, "-") { + sign, digits = "-", digits[1:] + } + if len(digits) <= 3 { + return sign + digits + } + lead := len(digits) % 3 + if lead == 0 { + lead = 3 + } + var out strings.Builder + out.Grow(len(digits) + (len(digits)-1)/3) + out.WriteString(digits[:lead]) + for i := lead; i < len(digits); i += 3 { + out.WriteByte(' ') + out.WriteString(digits[i : i+3]) + } + return sign + out.String() +} + // Percent divides before multiplying where it has to, so a very large run does // not wrap on the way to a number between nought and a hundred. // diff --git a/internal/guard/smallestaccepted_test.go b/internal/guard/smallestaccepted_test.go index cc4edb7..237b915 100644 --- a/internal/guard/smallestaccepted_test.go +++ b/internal/guard/smallestaccepted_test.go @@ -48,11 +48,17 @@ func smallestPrinted(t *testing.T, id string) int64 { t.Fatalf("formats %s does not print a minimum on its first line: %q", id, first) } rest := first[i+len(marker):] - end := strings.IndexByte(rest, ' ') + // Read up to the unit rather than up to the first space, and take the + // spaces out of what is left. The digits are grouped in threes as of + // 2026-09-08, so the count itself now CONTAINS spaces - and this guard read + // "1 220 B" as one byte and then reported that the tool refuses the minimum + // it advertises. It was right about what it saw and wrong about what it + // meant, which is what a parser splitting on the wrong thing always is. + end := strings.Index(rest, " B") if end < 0 { t.Fatalf("the minimum is not followed by a unit: %q", first) } - n, err := strconv.ParseInt(rest[:end], 10, 64) + n, err := strconv.ParseInt(strings.ReplaceAll(rest[:end], " ", ""), 10, 64) if err != nil { t.Fatalf("the minimum is not a number: %q", first) } From f23b332d779321c0ff0f5a82572bfab62f82bccc Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 8 Sep 2026 09:20:30 +0200 Subject: [PATCH 2/4] fix: teach three guards the grouped spelling they now read CI went red on eight tests, all of them guards reading a byte count out of the command line's own output. I had written in this branch's own message that a change to a human sentence has a blast radius equal to the number of things parsing it, and then shipped without running the suite, so the branch proved its own point. Three places, three different shapes: - sizeText built the expected text with strconv.FormatInt, so it looked for "36415" in a report saying "36 415 B" and found nothing. It asks core.ExactBytes now, WITH the unit - which is stronger than what it replaced rather than merely equal, because bare digits could match inside a longer number and "36415" does match in "136415". - The boundary announcement is pinned as literal text, so it moves to the grouped spelling. - The sizes a person writes are pinned as literal text as well, and deliberately not asked of core.ExactBytes: that guard is over what a PERSON reads, so the spelling is half of what it holds, and a guard built from the same function the program prints with cannot tell the two apart. Every expected string was checked against what the program actually prints rather than worked out by hand: 1 610 612 736 B, 10 485 760 B, 1 048 576 B, 716 800 B, 0 B, 15 728 640 B. One subtest cannot run on this machine and it is not this change: 1.5gib is refused with exit 6, the free space code, because the disk has 1.1 GB left. The tool is behaving correctly and the runner has room. Co-Authored-By: Claude Opus 5 --- internal/guard/boundaryunits_test.go | 2 +- internal/guard/contains_test.go | 11 +++++++++-- internal/guard/smallfixes_test.go | 13 +++++++++---- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/internal/guard/boundaryunits_test.go b/internal/guard/boundaryunits_test.go index ead2004..dc15aaf 100644 --- a/internal/guard/boundaryunits_test.go +++ b/internal/guard/boundaryunits_test.go @@ -77,7 +77,7 @@ func TestABoundaryRunSaysTheNumberItBuiltAround(t *testing.T) { // The three file lines carry the byte count as well, so asking whether the // number appears at all left this green when the announcement lost it - // which the mutation runner said out loud on 2026-08-18. - if !strings.Contains(errOut, `boundary "files" around 15728640 B`) { + if !strings.Contains(errOut, `boundary "files" around 15 728 640 B`) { t.Errorf("the run built a set around 15728640 B and never says so.\n"+ "Reason: 15mb can be read two ways, and printing the byte count is what lets somebody\n"+ "whose system meant 15000000 see it before a byte is written.\nWhat it said:\n%s", out) diff --git a/internal/guard/contains_test.go b/internal/guard/contains_test.go index 5131bb6..1bb8ef3 100644 --- a/internal/guard/contains_test.go +++ b/internal/guard/contains_test.go @@ -4,11 +4,11 @@ import ( stdzip "archive/zip" "os" "path/filepath" - "strconv" "strings" "testing" "github.com/donislawdev/TestingFilesGenerator/internal/cli" + "github.com/donislawdev/TestingFilesGenerator/internal/core" ) // onlyArchive is the single .zip the run produced. Failing when there is not @@ -56,7 +56,14 @@ func archiveMembers(t *testing.T, path string) ([]string, []int64) { return names, sizes } -func sizeText(n int64) string { return strconv.FormatInt(n, 10) } +// sizeText is a number of bytes as the command line writes it. +// +// It carries the unit as well as the digits, and both halves are load bearing. +// The digits are grouped in threes since 2026-09-08, so a guard holding +// strconv.FormatInt found nothing at all in a report saying "36 415 B" - and +// the bare digits it used to look for could match INSIDE a longer number, +// which "36415" in "136415" does. +func sizeText(n int64) string { return core.ExactBytes(n) } // "an archive holds real files of other formats" is the feature docs/ // MVP-FORMATS.md 5.7 calls the key one, and the difference between this tool diff --git a/internal/guard/smallfixes_test.go b/internal/guard/smallfixes_test.go index e1c5f09..571a3d8 100644 --- a/internal/guard/smallfixes_test.go +++ b/internal/guard/smallfixes_test.go @@ -40,11 +40,16 @@ func TestASizeIsWrittenTheWayAPersonWritesIt(t *testing.T) { // The decimal point stays. 1.5gib is a real thing people write, and the // fix must not reach it. + // Written out grouped, the way the command line prints them since + // 2026-09-08, rather than asked of core.ExactBytes. This is a guard over + // what a PERSON reads, so the spelling is half of what it pins - and a + // guard built from the same function the program prints with cannot tell + // the two apart. accepted := map[string]string{ - "1.5gib": "1610612736", - "10mb": "10485760", - "1048576": "1048576", - "700kB": "716800", + "1.5gib": "1 610 612 736", + "10mb": "10 485 760", + "1048576": "1 048 576", + "700kB": "716 800", "0": "0", } for size, want := range accepted { From b0e19561fdef0142b0636888c1debf04da3c2657 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 8 Sep 2026 11:42:01 +0200 Subject: [PATCH 3/4] format: an SVG drawing can be any size you ask for Two new settings on svg: width and height, a whole number of pixels from 1 to 20000 each. They default to the 800 by 600 these drawings have always been, so a recipe that says nothing gets the same bytes it got before. tfg generate --format svg --size 20kb --set width=1920 --set height=1080 There is no joint limit on the two, unlike the picture formats, because nothing is drawn into pixels here - the file only says how big it is. That makes a small file that claims to be enormous, which is the point: a 3 kB drawing declaring 20000 by 20000 asks whether whatever opens it has a limit on picture size and not only on file size. The ceiling is not the renderer's. Measured headless: Inkscape draws 4295 megapixels in 165 s without complaint. The line worth crossing belongs to a reader instead - Pillow refuses an image over 89478485 px as a decompression bomb - and 20000 per axis reaches 400 megapixels, so a set can hold files on both sides of it. A drawing shorter than 57 pixels has no room for the label along its bottom edge. It is still produced and still named, and the run says which files those were. Two defects the settings would otherwise have introduced, both invisible while the dimensions were constants: - the shape code assumed a margin of up to eighty units, and rand.IntN panics on a non-positive argument, so a narrow canvas would have been a panic on a value that looks entirely legal; - a shape whose lower edge ran past the drawing would paint over the label, which is the defect the strip along the bottom was added for in the first place. Both clamps are inert at 800 by 600 - the widest shape lands two units short - so the stored byte hashes are unchanged and the guards press the sizes where the clamps are not. Four new guards, five new mutations, and four existing SVG mutations repointed after the constants they aimed at became functions. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 20 ++ README.md | 3 +- internal/format/svgfile/svg.go | 313 +++++++++++++++++++++------ internal/guard/parity_test.go | 7 + internal/guard/svgdimensions_test.go | 253 ++++++++++++++++++++++ web/public/formats/index.html | 10 + web/public/pl/formaty/index.html | 10 + 7 files changed, 554 insertions(+), 62 deletions(-) create mode 100644 internal/guard/svgdimensions_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 6db6e9f..0651326 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,26 @@ because it turns other people's test suites red. ### Added +- **SVG drawings can be any size you ask for.** Two new settings on `svg`: + `width` and `height`, both a whole number of pixels from 1 to 20000. They + default to the 800 by 600 these drawings have always been, so a recipe that + says nothing gets the same bytes it got before. + + ``` + tfg generate --format svg --size 20kb --set width=1920 --set height=1080 + ``` + + There is no joint limit on the two, unlike the picture formats, because + nothing is drawn into pixels here - the file only says how big it is. That + makes a small file that claims to be enormous, which is the point: a 3 kB + drawing declaring 20000 by 20000 asks whether whatever opens it has a limit + on picture size and not only on file size. Pillow, for one, refuses to open + the result. + + A drawing shorter than 57 pixels has no room for the label along its bottom + edge. It is still produced and still named, and the run says which files + those were. + - **Text and Markdown files can be written in UTF-16, with or without a byte order mark.** Two new settings on `txt` and `md`: `encoding`, which takes `utf-8`, `utf-16le` or `utf-16be`, and `bom`, which is `true` or `false`. diff --git a/README.md b/README.md index 7d064c7..e2090e6 100644 --- a/README.md +++ b/README.md @@ -478,7 +478,8 @@ recipe. `tfg formats ` prints the allowed range or list for each: | `log` | `entry_format`, `timestamps`, `rate`, `methods`, `status_mix`, `level_mix`, `ip_version`, `line_ending` | | `txt`, `md` | `encoding`, `bom` | | `json` | `formatting` | -| `xml`, `html`, `svg` | none | +| `svg` | `width`, `height` | +| `xml`, `html` | none | ``` tfg generate --format jpg --size 500kb --set width=1920 --set height=1080 --set quality=85 diff --git a/internal/format/svgfile/svg.go b/internal/format/svgfile/svg.go index 4fbb64f..b277d44 100644 --- a/internal/format/svgfile/svg.go +++ b/internal/format/svgfile/svg.go @@ -37,47 +37,149 @@ import ( const ( generatorVersion = "1" - width = 800 - height = 600 + defaultWidth = 800 + defaultHeight = 600 - // textBand is the strip along the bottom edge that shapes stay out of, so + // The range the two dimensions accept. + // + // The upper end is the same number the nine picture formats use, and that + // is deliberate rather than lazy: somebody who has learnt "width up to + // 20000" for PNG does not learn a second number here. What is NOT carried + // over is their joint ceiling of 40 megapixels, because the reason for it + // does not exist here - a picture format holds the raster in memory while + // it encodes, and this one writes text. + // + // Measured 2026-09-08, a document of this shape rendered headless by + // Inkscape 1.4.4 and read back with Pillow: + // + // 10000x8000 80 Mpx 5.3 s 339 kB + // 20000x20000 400 Mpx 25.6 s 1.6 MB + // 32000x32000 1024 Mpx 43.6 s 4.1 MB + // 65536x65536 4295 Mpx 165.3 s 17 MB + // + // So the renderer does not set this ceiling - it never broke. The line + // worth crossing belongs to a reader instead: Pillow refuses an image over + // PIL.Image.MAX_IMAGE_PIXELS, measured at 89478485 px, as a decompression + // bomb. 20000 per axis reaches 400 Mpx, four and a half times over that + // line, so a set can hold files on both sides of it. That is how the CSV + // column ceiling was chosen too - above the point where a real reader + // starts to say no, not as high as the arithmetic allows. + minDimension = 1 + maxDimension = 20000 + + // TextBand is the strip along the bottom edge that shapes stay out of, so // the two lines of text below it are read against plain background. // // Without it about one shape in ten landed on the label - measured on a // small and a large file, 11.0% and 10.3% - and the label is the one thing // in the file that says what the file is. A drawing is still a drawing with // a margin. A label with a circle through it is not a label. - textBand = 56 - drawHeight = height - textBand + TextBand = 56 + + // Width and Height name the two settings. Exported so that a guard presses + // the key this format actually declares rather than a string spelled twice, + // which is the same reason jsonfile exports the name of its layout setting. + Width = "width" + Height = "height" declaration = `` + "\n" - rootOpen = `` + "\n" rootClose = "\n" - // The closing record is a text element, so the drawing ends with something - // that can be stretched to any length without changing what it is. - // - // It gets its own baseline. Sharing one with the identity label meant the - // closing text, written last, painted straight over it - the label was - // there in the bytes and unreadable on screen. Nothing caught that: the - // size was exact, the file parsed, and a renderer still drew a full page of - // shapes. - textOpen = `` - textY = "566" textClose = "\n" - // labelOpen carries the identity label along the bottom edge. It is the - // same width as textOpen, so the smallest file this format can produce does - // not move. - labelOpen = `` - labelY = "584" - tailLast = textClose + rootClose - - // fixedWidth is every literal byte of the closing record. - fixedWidth = len(textOpen) + len(tailLast) ) +// rootOpen is the opening tag for a drawing of these dimensions. +// +// It used to be a constant with 800 and 600 written into it, which is why the +// minimum below was a constant too. Both now depend on the dimensions asked +// for: "width=\"20000\"" is three bytes longer than "width=\"800\"", and the +// baseline of a text element near the bottom edge of a tall drawing is a +// longer number as well. +func rootOpen(w, h int) string { + return fmt.Sprintf( + ``+"\n", + w, h, w, h) +} + +// textOpen opens the closing record: a text element, so the drawing ends with +// something that can be stretched to any length without changing what it is. +// +// It gets its own baseline. Sharing one with the identity label meant the +// closing text, written last, painted straight over it - the label was there +// in the bytes and unreadable on screen. Nothing caught that: the size was +// exact, the file parsed, and a renderer still drew a full page of shapes. +func textOpen(h int) string { + return fmt.Sprintf(``, h-34) +} + +// labelOpen carries the identity label along the bottom edge, on the other +// baseline of the two. +func labelOpen(h int) string { + return fmt.Sprintf(``, h-16) +} + +// labelFits says whether the band along the bottom has room to show the label. +// +// Below this the file is still produced and still named - it simply carries no +// visible label, and says so. That is the same answer the picture formats give +// for a drawing too small to write on, and reusing their sentence is the point: +// a second wording for one situation is a second thing to keep true. +func labelFits(h int) bool { return h > TextBand } + +// drawHeightFor is the strip shapes are drawn in. +// +// At least one row, always. A drawing shorter than the text band has no room +// for the band, and the band is a courtesy to the label rather than a +// structural part of the document. +func drawHeightFor(h int) int { + if d := h - TextBand; d > 0 { + return d + } + return 1 +} + +// span keeps an argument to IntN positive. +// +// The shape code below assumes a margin of up to eighty units, which was safe +// for as long as the canvas was a constant 800 by 600. It is not safe now: +// measured 2026-09-08 by running it, IntN(0) and IntN(-30) both panic with +// "invalid argument to IntN", so "--set width=50" would have been a panic on a +// value that looks entirely legal. A generator panic costs one file rather +// than the process - there is a guard for that - but a panic is not an answer +// to a legal setting. +// +// At 800 by 600 every argument is already positive, so this changes no byte of +// any file this tool has produced. +func span(n int) int { + if n < 1 { + return 1 + } + return n +} + +// fit keeps a shape's lower edge out of the text band on any canvas. +// +// Arithmetic, not caution: at the default dimensions this can never bind. The +// widest rect starts at 719 and reaches 798 of 800, the tallest at 463 and +// reaches 542 of 544, and the largest circle and ellipse both reach 542 as +// well. Two units of slack on every axis, so the clamp is inert at 800 by 600 +// and the stored hashes prove it. +// +// room is always at least one, so this never returns a shape of no size. Each +// caller subtracts a coordinate drawn from span(drawHeight-margin) from +// drawHeight: above the margin that leaves the margin itself, and at or below +// it span returns one, the coordinate is nought and the room is the whole +// drawing. There is no third case. +// +// That claim was a guarded branch here until it was measured. A panic put in +// its place did not fire once across the whole canvas sweep, so the branch was +// a defence nothing could turn red - the eighth of its kind removed from this +// codebase. It is written down instead, which is what a claim nothing can +// contradict is worth. +func fit(extent, room int) int { return min(extent, room) } + func init() { format.Register(format.Descriptor{ ID: "svg", @@ -89,7 +191,16 @@ func init() { // rectangle. That is a shape request rather than a byte count, and it // arrives with the shape count property. The minimum here is the // declaration, the root and one whole record. - MinBytes: minimumBytes(), + // + // It is the minimum at the DEFAULT dimensions, and only that. Larger + // numbers in the root element and in a text baseline make a longer + // document, so the floor moves with the settings - the same shape as + // the JSON layouts, where the registry states the default layout's + // minimum and each of the others answers for its own. Nobody has to + // keep a second number in step: Plan refuses with the figure for the + // dimensions actually asked for, and Descriptor.SmallestAccepted asks + // Plan rather than reading anything declared here. + MinBytes: minimumBytes(defaultWidth, defaultHeight), Padding: format.PaddingChannel{ Name: "the text of the closing label", @@ -101,10 +212,25 @@ func init() { // rather than a comment. Label: format.LabelVisible, Oracle: "inkscape", - // Dimensions, shape counts, gradients, fonts, embedded rasters and SMIL - // come later. Declaring none now makes a recipe asking for them fail - // loudly. - Properties: nil, + // Shape counts, gradients, fonts, embedded rasters and SMIL come + // later. Declaring none of them makes a recipe asking for one fail + // loudly rather than quietly. + Properties: []format.Property{ + { + Name: Width, Kind: format.PropertyInt, + Min: minDimension, Max: maxDimension, Unit: "pixels", + Default: strconv.Itoa(defaultWidth), + Detail: "How wide the drawing says it is. Nothing is drawn into pixels here, " + + "so a large number costs a few bytes in the file and a great deal of memory in whatever opens it.", + }, + { + Name: Height, Kind: format.PropertyInt, + Min: minDimension, Max: maxDimension, Unit: "pixels", + Default: strconv.Itoa(defaultHeight), + Detail: "How tall the drawing says it is. The label sits along the bottom edge, " + + "so a drawing shorter than that strip carries no visible label.", + }, + }, GeneratorVersion: generatorVersion, Generator: generator{}, }) @@ -115,10 +241,48 @@ type generator struct{} type memo struct { labelLine string // includes the trailing newline, empty when absent seed uint64 + width int + height int +} + +// dimension reads one of the two size settings. +// +// The registry has already refused anything outside the declared range by the +// time a run reaches here, so the bounds below are a backstop for a caller +// that reaches the generator directly - the same belt the page count of PDF +// wears, and for the same reason. +func dimension(props map[string]string, name string, fallback int) (int, error) { + raw, ok := props[name] + if !ok || raw == "" { + return fallback, nil + } + n, err := strconv.Atoi(raw) + if err != nil { + return 0, &format.PropertyValueError{ + Format: "svg", Key: name, Value: raw, + Reason: "it has to be a whole number of pixels", + } + } + if n < minDimension || n > maxDimension { + return 0, &format.PropertyValueError{ + Format: "svg", Key: name, Value: raw, + Reason: fmt.Sprintf("it has to be between %d and %d", minDimension, maxDimension), + } + } + return n, nil } func (generator) Plan(r format.Request) (format.Plan, error) { - min := minimumBytes() + w, err := dimension(r.Properties, Width, defaultWidth) + if err != nil { + return format.Plan{}, err + } + h, err := dimension(r.Properties, Height, defaultHeight) + if err != nil { + return format.Plan{}, err + } + + min := minimumBytes(w, h) if r.Bytes < min { return format.Plan{}, &format.BelowMinimumError{ Format: "SVG", @@ -136,18 +300,28 @@ func (generator) Plan(r format.Request) (format.Plan, error) { Properties: map[string]any{ "encoding": "utf-8", "line_ending": "lf", - "width": width, - "height": height, - "view_box": "0 0 800 600", + Width: w, + Height: h, + "view_box": fmt.Sprintf("0 0 %d %d", w, h), }, } - m := memo{seed: r.Seed} + m := memo{seed: r.Seed, width: w, height: h} if r.Label { - line := labelOpen + core.Label("svg", r.Bytes, r.Seed) + textClose - if int64(len(line))+minimumBytes() <= r.Bytes { + switch line := labelOpen(h) + core.Label("svg", r.Bytes, r.Seed) + textClose; { + case !labelFits(h): + // Room on the page rather than room in the byte count, so it is a + // separate sentence from the one below. Both leave the file named + // by its own name and by the manifest. + p.Notes = append(p.Notes, format.Note{ + Code: "label_omitted", + Detail: fmt.Sprintf( + "The drawing is %d px tall and the label needs the %d px strip along the bottom, so this file carries no visible label. Its name and the manifest still identify it.", + h, TextBand), + }) + case int64(len(line))+min <= r.Bytes: m.labelLine = line - } else { + default: p.Notes = append(p.Notes, format.Note{ Code: "label_omitted", Detail: fmt.Sprintf( @@ -168,24 +342,37 @@ func (generator) Write(ctx context.Context, w io.Writer, p format.Plan) error { return fmt.Errorf("svg: the plan was not produced by this generator") } - head := declaration + rootOpen + m.labelLine + head := declaration + rootOpen(m.width, m.height) + m.labelLine if err := core.WriteAll(w, []byte(head)); err != nil { return err } rng := core.NewRand(m.seed) - return core.FillRecords(ctx, w, rng, p.Bytes-int64(len(head)), shapes{}) + return core.FillRecords(ctx, w, rng, p.Bytes-int64(len(head)), shapesFor(m.width, m.height)) } // shapes builds the drawing. A natural record is one shape, and the closing // record is a text label stretched to land the byte count. -type shapes struct{} +// +// It carries the canvas rather than reading package constants, because the +// canvas is a setting now. Nothing else changed about what it draws. +type shapes struct { + width int + drawHeight int + // open is the closing record's opening tag, whose baseline depends on how + // tall the drawing is. + open string +} + +func shapesFor(w, h int) shapes { + return shapes{width: w, drawHeight: drawHeightFor(h), open: textOpen(h)} +} // Shortest is the smallest closing record: the label element with no text at // all, plus the bytes that close the root. -func (shapes) Shortest() int64 { return int64(fixedWidth) } +func (s shapes) Shortest() int64 { return int64(len(s.open) + len(tailLast)) } -func (shapes) Append(dst []byte, rng *rand.Rand) []byte { +func (s shapes) Append(dst []byte, rng *rand.Rand) []byte { // A line is drawn with a stroke and a closed shape is filled. Each branch // says which it wants, because a shape painted the wrong way is invisible // and the size never notices. @@ -193,39 +380,43 @@ func (shapes) Append(dst []byte, rng *rand.Rand) []byte { switch rng.IntN(4) { case 0: + x := rng.IntN(span(s.width - 80)) dst = append(dst, ` edge { + edge = v + } + shapes++ + } + num := func(b []byte) int { + n, _ := strconv.Atoi(string(b)) + return n + } + for _, m := range rectRe.FindAllSubmatch(doc, -1) { + note(num(m[1]) + num(m[2])) + } + for _, m := range circleRe.FindAllSubmatch(doc, -1) { + note(num(m[1]) + num(m[2])) + } + for _, m := range ellipseRe.FindAllSubmatch(doc, -1) { + note(num(m[1]) + num(m[2])) + } + for _, m := range lineRe.FindAllSubmatch(doc, -1) { + y1, y2 := num(m[1]), num(m[2]) + if y2 > y1 { + y1 = y2 + } + note(y1) + } + return edge, shapes +} + +// TestNoSvgShapeReachesIntoTheStripTheLabelSitsIn presses the clamp. +// +// The strip is asked of the format rather than written down here, so the two +// cannot drift: a wider strip would move this bound with it. +func TestNoSvgShapeReachesIntoTheStripTheLabelSitsIn(t *testing.T) { + for _, c := range svgCanvases { + w, h := c[0], c[1] + t.Run(fmt.Sprintf("%dx%d", w, h), func(t *testing.T) { + doc := drawSVG(t, 8192, svgCanvas(w, h)) + edge, shapes := lowestSVGEdge(doc) + if shapes == 0 { + t.Fatalf("%dx%d: read no shapes at all - this case proves nothing, "+ + "and two empty lists are equal", w, h) + } + room := h - svgfile.TextBand + if room < 1 { + room = 1 + } + if edge > room { + t.Errorf("%dx%d: a shape reaches %d and the drawing is %d deep before the "+ + "%d px strip the label sits in - a label with a circle through it is not a label", + w, h, edge, room, svgfile.TextBand) + } + }) + } +} + +// TestSayingTheSvgCanvasOutLoudChangesNoByte is the promise a new setting makes +// to every recipe written before it existed. +// +// Silence and stating the default are two different journeys through the +// parser, the registry and the generator, and they are allowed to disagree by +// accident. This is the one place that says they must not. +func TestSayingTheSvgCanvasOutLoudChangesNoByte(t *testing.T) { + d, err := format.Get("svg") + if err != nil { + t.Fatal(err) + } + + // The default is read from the declaration rather than written here. A + // number copied into a test is a number that stops being the default + // without anything saying so. + spoken := map[string]string{} + for _, p := range d.Properties { + if p.Name != svgfile.Width && p.Name != svgfile.Height { + continue + } + if p.Default == "" { + t.Fatalf("%s declares no default, so silence has nothing to be equal to", p.Name) + } + spoken[p.Name] = p.Default + } + if len(spoken) != 2 { + t.Fatalf("expected width and height to be declared, found %d of them", len(spoken)) + } + + for _, size := range []int64{194, 1024, 20480} { + silent := drawSVG(t, size, nil) + aloud := drawSVG(t, size, spoken) + if !bytes.Equal(silent, aloud) { + t.Errorf("%d B: saying %v out loud produced different bytes from leaving it out", + size, spoken) + } + } +} + +// TestASvgTooShortForTheLabelSaysSo is the untouchable rule about silence, +// applied to the one thing a drawing can lose by being small. +func TestASvgTooShortForTheLabelSaysSo(t *testing.T) { + d, err := format.Get("svg") + if err != nil { + t.Fatal(err) + } + for _, c := range []struct { + height int + labels bool + }{{1, false}, {40, false}, {svgfile.TextBand, false}, {svgfile.TextBand + 1, true}, {600, true}} { + p, err := d.Generator.Plan(format.Request{ + Bytes: 8192, Seed: 7741, Label: true, Properties: svgCanvas(400, c.height), + }) + if err != nil { + t.Fatalf("planning at height %d: %v", c.height, err) + } + got, _ := p.Properties[format.PropertyLabelEmbedded].(bool) + if got != c.labels { + t.Errorf("height %d: label embedded is %v, expected %v", c.height, got, c.labels) + } + said := false + for _, n := range p.Notes { + if n.Code == "label_omitted" { + said = true + } + } + if !c.labels && !said { + t.Errorf("height %d: no visible label and the run says nothing about it", c.height) + } + if c.labels && said { + t.Errorf("height %d: the label is there and the run apologises for it anyway", c.height) + } + } +} diff --git a/web/public/formats/index.html b/web/public/formats/index.html index 6e30225..57badba 100644 --- a/web/public/formats/index.html +++ b/web/public/formats/index.html @@ -510,6 +510,16 @@

Settings each format accepts

slides 1 - 500 slides + + svg + width + 1 - 20000 pixels + + + + height + 1 - 20000 pixels + targz entries diff --git a/web/public/pl/formaty/index.html b/web/public/pl/formaty/index.html index 2ed90a6..3d0a921 100644 --- a/web/public/pl/formaty/index.html +++ b/web/public/pl/formaty/index.html @@ -510,6 +510,16 @@

Ustawienia, które przyjmuje każdy format

slides 1 - 500 slajdów + + svg + width + 1 - 20000 pikseli + + + + height + 1 - 20000 pikseli + targz entries From c69791cdb0bd93a966067f5f1d263bbf3a36c8eb Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 8 Sep 2026 11:49:45 +0200 Subject: [PATCH 4/4] guard: compare the decoder's end of input with errors.Is golangci-lint's errorlint rule refuses == against a sentinel error, because it fails on a wrapped one. Reproduced locally with the pinned v2.13.2 rather than read off the runner. Co-Authored-By: Claude Opus 5 --- internal/guard/svgdimensions_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/guard/svgdimensions_test.go b/internal/guard/svgdimensions_test.go index e650aae..e2422db 100644 --- a/internal/guard/svgdimensions_test.go +++ b/internal/guard/svgdimensions_test.go @@ -24,6 +24,7 @@ import ( "bytes" "context" "encoding/xml" + "errors" "fmt" "io" "regexp" @@ -92,7 +93,7 @@ func TestASvgCanvasSmallerThanItsOwnMarginsStillDraws(t *testing.T) { dec := xml.NewDecoder(bytes.NewReader(doc)) for { _, err := dec.Token() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if err != nil {