From 9ebda53490a713eb9b99ec6437b6d4d4ffaa6626 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Fri, 25 Sep 2026 15:19:59 +0200 Subject: [PATCH] preset: eject -o writes the recipe to a file, and a UTF-16 recipe says why it is refused The help of tfg preset eject said "> my.yaml", and Windows PowerShell 5.1 saves that as UTF-16, which tfg then refused with advice to save the file as UTF-8 - to somebody who had saved nothing. Every way through PowerShell 5.1, Out-File -Encoding utf8 included, first decodes the output in the console's code page, so on a stock console a name in Polish or Korean arrived changed or not at all. Measured in both PowerShells: seven ways, and only PowerShell 7 keeps the bytes of a redirect. -o writes byte for byte what eject prints, in every shell. The name is claimed first and the content put in place by a rename, so a file already there - it may be a recipe somebody edited - is refused and left as it is, and a write that fails takes its claim back. -o "" and -o - are usage errors. A recipe starting with a UTF-16 byte order mark is refused with its own sentence: what the file is, that PowerShell 5.1 writes it for >, and the way round it. It is still not read: by then PowerShell may already have changed its letters outside ASCII, and reading it would turn a refusal into names quietly different from the ones asked for. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 18 ++++ README.md | 7 +- internal/cli/errors.go | 9 ++ internal/cli/presetcmd.go | 97 ++++++++++++++++--- internal/guard/ejectfile_test.go | 160 +++++++++++++++++++++++++++++++ internal/recipe/canonical.go | 11 +++ internal/recipe/errors.go | 4 + internal/recipe/recipe.go | 11 +++ 8 files changed, 303 insertions(+), 14 deletions(-) create mode 100644 internal/guard/ejectfile_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index c1d53389..54a95236 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -307,6 +307,16 @@ because it turns other people's test suites red. ### Added +- **`tfg preset eject -o my.yaml` writes the recipe to a file itself.** + The help used to say `> my.yaml`, and Windows PowerShell 5.1 saves that as + UTF-16, which `tfg` then refused to read. Piping through + `Out-File -Encoding utf8` was no way round it: PowerShell 5.1 first reads + the output in the console's code page, which changes letters outside ASCII, + so a name in Polish or Korean arrived different or not at all. `-o` writes + byte for byte what would have been printed, in every shell. A file already + at that name is refused and left as it is, because it may be a recipe you + edited. `> my.yaml` still works in cmd, bash and PowerShell 7. + - **A preset for unusual file names: `filename-handling`.** It answers "will my system store, show and give back a file name it did not expect?" with fifty names in seven groups: scripts from Polish to Korean, names that look @@ -520,6 +530,14 @@ because it turns other people's test suites red. ### Fixed +- **A recipe saved as UTF-16 is refused with the reason and the way round + it.** The refusal told you to save the file as UTF-8, when the usual way to + get UTF-16 is not saving at all but `>` in Windows PowerShell 5.1. It now + says the file is UTF-16, where that comes from, and to use + `tfg preset eject -o my.yaml` or `>` in PowerShell 7, cmd or bash. + The exit code is still `3`, and such a file is still not read: by then + PowerShell may already have changed its letters outside ASCII. + - **A report shows a character nobody can see in a file name as an escape.** `verify`, `cleanup`, the notes of a run, every error message and the refusals in the window printed such a character as it was, in a file name diff --git a/README.md b/README.md index 81ed10fb..15645801 100644 --- a/README.md +++ b/README.md @@ -334,9 +334,14 @@ is **valid**, which is what `tfg validate` is for. ``` tfg preset list [--json] what this build offers tfg preset show [--json] what it takes and what it would produce -tfg preset eject > my.yaml the recipe it stands for, to edit +tfg preset eject -o my.yaml the recipe it stands for, to edit ``` +`-o` writes the recipe byte for byte as it would be printed, and refuses a file +that is already there. `tfg preset eject > my.yaml` does the same in cmd, +bash and PowerShell 7. Windows PowerShell 5.1 saves it as UTF-16, which `tfg` +refuses to read, so use `-o` there. + ### `tfg formats` ``` diff --git a/internal/cli/errors.go b/internal/cli/errors.go index 9ed1b352..1dcc2c9f 100644 --- a/internal/cli/errors.go +++ b/internal/cli/errors.go @@ -78,6 +78,15 @@ func inOurWords(err error) string { return "stopped before it finished, because the time allowed for it ran out." } + // A recipe Windows PowerShell 5.1 saved with ">" (O245). The recipe + // package says what the file is and why. The way round it is a flag, and a + // flag is this surface's to name - the packages under both surfaces never + // spell one (O79). + var syntax *recipe.SyntaxError + if errors.As(err, &syntax) && syntax.UTF16 { + return err.Error() + ". Have tfg write the file itself with tfg preset eject -o my.yaml, or redirect in PowerShell 7, cmd or bash, which keep the bytes as they are" + } + var errno syscall.Errno if !errors.As(err, &errno) { return err.Error() diff --git a/internal/cli/presetcmd.go b/internal/cli/presetcmd.go index da872668..ea26478d 100644 --- a/internal/cli/presetcmd.go +++ b/internal/cli/presetcmd.go @@ -10,9 +10,11 @@ package cli import ( "context" + "errors" "flag" "fmt" "io" + "os" "strings" "github.com/donislawdev/TestingFilesGenerator/internal/core" @@ -51,7 +53,7 @@ func presetUsage(w io.Writer) { Usage: tfg preset list what this build offers tfg preset show what it takes and what it would produce - tfg preset eject > my.yaml the recipe it stands for, to edit + tfg preset eject -o my.yaml the recipe it stands for, to edit A preset is a recipe with a name. Ejecting one gives back an ordinary recipe file, so nothing here is a closed box. @@ -65,10 +67,12 @@ Run "tfg generate --preset " to produce the files. // The id has to be read before parsing, because the parameters of the preset // are flags and there is no way to register them until it is known which they // are. -// asJSON is filled in for the operations that have a machine readable form and -// nil for the one that does not - a recipe is already machine readable, and a -// second encoding of it would be a second thing to keep in step. -func presetFlagSet(name string, args []string, out, errOut io.Writer, usage func(io.Writer), asJSON *bool) ( +// +// own registers the operation's own flags: --json for show, which has a +// machine readable form, and -o for eject, which writes a file. Eject has no +// --json - a recipe is already machine readable, and a second encoding of it +// would be a second thing to keep in step. +func presetFlagSet(name string, args []string, out, errOut io.Writer, usage func(io.Writer), own func(*flag.FlagSet)) ( *preset.Expansion, int) { fs := flag.NewFlagSet("preset "+name, flag.ContinueOnError) @@ -80,9 +84,7 @@ func presetFlagSet(name string, args []string, out, errOut io.Writer, usage func } // Registered before the parameters, so a preset declaring one called json // is caught by the collision check rather than by the flag package panicking. - if asJSON != nil { - fs.BoolVar(asJSON, "json", false, "write the answer as JSON to standard output") - } + own(fs) id, rest := splitLeadingPath(args) if id == "" { @@ -189,7 +191,9 @@ Usage: `) } var asJSON bool - expanded, code := presetFlagSet("show", args, out, errOut, usage, &asJSON) + expanded, code := presetFlagSet("show", args, out, errOut, usage, func(fs *flag.FlagSet) { + fs.BoolVar(&asJSON, "json", false, "write the answer as JSON to standard output") + }) if expanded == nil { return code } @@ -256,26 +260,93 @@ func presetEject(args []string, out, errOut io.Writer) int { Prints an ordinary recipe file. Edit it, commit it, run it with tfg generate - from here on it is yours and nothing about it is special. -The recipe goes to standard output and everything else to standard error, so -"tfg preset eject size-boundaries > my.yaml" gives a clean file. +With -o the recipe is written to that file, byte for byte what would have been +printed. A file already at that name is refused and left as it is, because it +may be a recipe somebody ejected and then edited. + +Without -o the recipe goes to standard output and everything else to standard +error, so "tfg preset eject size-boundaries > my.yaml" gives a clean file in +cmd, bash and PowerShell 7. Windows PowerShell 5.1 saves it as UTF-16, which +tfg refuses to read, so use -o there. Usage: + tfg preset eject size-boundaries -o my.yaml + tfg preset eject size-boundaries --limit 20mb --format png -o my.yaml tfg preset eject size-boundaries > my.yaml - tfg preset eject size-boundaries --limit 20mb --format png > my.yaml `) } - expanded, code := presetFlagSet("eject", args, out, errOut, usage, nil) + var to fileFlag + expanded, code := presetFlagSet("eject", args, out, errOut, usage, func(fs *flag.FlagSet) { + fs.Var(&to, "o", "write the recipe to this file rather than to standard output. A file already there is refused") + }) if expanded == nil { return code } + if to.set && (to.name == "" || to.name == "-") { + fmt.Fprintf(errOut, "tfg: -o takes the name of the file to write the recipe to, and %q is not one. Leave -o out and the recipe goes to standard output.\n", to.name) + return ExitUsage + } // The note goes to the error channel. The recipe is the data here, and a // sentence about a number we chose has no business inside a file somebody // is about to commit. sayNotes(expanded.Notes(), errOut) + if to.set { + return writeEjected(to.name, expanded.Source, errOut) + } if _, err := out.Write(expanded.Source); err != nil { fmt.Fprintf(errOut, "tfg: cannot write the recipe: %s\n", describeError(err)) return ExitIO } return ExitOK } + +// fileFlag is a file name given as a flag, and whether it was given at all. An +// empty name typed on purpose is a mistake to report rather than the default. +type fileFlag struct { + name string + set bool +} + +func (f *fileFlag) String() string { return f.name } + +func (f *fileFlag) Set(s string) error { + f.name, f.set = s, true + return nil +} + +// writeEjected puts the recipe into a file nobody holds. +// +// Written by the tool rather than left to the shell because of O245, measured +// on 2026-09-25: Windows PowerShell 5.1 saves "> my.yaml" as UTF-16, and every +// way through PowerShell - Out-File included - first decodes the output with +// the console's code page, which on a stock console changes every letter +// outside ASCII. Only the bytes the tool writes itself arrive as they are. +// +// Claimed first and then replaced whole. The claim is exclusive and does not +// follow a link (core.CreateNew), which is what keeps an edited recipe from +// being written over. The replacement goes through a temporary name and a +// rename (core.ReplaceFile), so a run stopped part way leaves an empty file or +// none rather than a recipe cut short - and a YAML file cut short can still +// read as a smaller recipe. +func writeEjected(path string, source []byte, errOut io.Writer) int { + f, err := core.CreateNew(path, 0o644) + if err != nil { + var taken *core.NameTakenError + if errors.As(err, &taken) { + fmt.Fprintf(errOut, "tfg: %s is already there, and -o does not write over a file - it may be a recipe somebody edited. Nothing was written. Choose another name, or remove that file first.\n", core.Shown(path)) + return ExitIO + } + fmt.Fprintf(errOut, "tfg: cannot write the recipe to %s: %s\n", core.Shown(path), describeError(err)) + return ExitIO + } + _ = f.Close() + if err := core.ReplaceFile(path, source); err != nil { + // Only the empty claim this call made is there to take back. + _ = os.Remove(path) + fmt.Fprintf(errOut, "tfg: cannot write the recipe to %s: %s\n", core.Shown(path), describeError(err)) + return ExitIO + } + fmt.Fprintf(errOut, "recipe: %s\n", core.Shown(path)) + return ExitOK +} diff --git a/internal/guard/ejectfile_test.go b/internal/guard/ejectfile_test.go new file mode 100644 index 00000000..a07858a3 --- /dev/null +++ b/internal/guard/ejectfile_test.go @@ -0,0 +1,160 @@ +package guard + +import ( + "bytes" + "encoding/binary" + "os" + "path/filepath" + "strings" + "testing" + "unicode/utf16" + + "github.com/donislawdev/TestingFilesGenerator/internal/cli" + "github.com/donislawdev/TestingFilesGenerator/internal/core" +) + +// O245, measured on 2026-09-25: the help of "tfg preset eject" said "> my.yaml", +// and Windows PowerShell 5.1 saves that as UTF-16, which the tool then refused. +// Every other way through PowerShell 5.1 - Out-File included - decodes the +// output with the console's code page first, so on a stock console the letters +// outside ASCII were changed before anything reached the file. The answer is +// that the tool writes the file itself, and that the refusal of a UTF-16 file +// says where it came from. docs/O245-EJECT-2026-09-25.md. + +// ejectedPreset has names in Polish, Korean and emoji, so a byte that moved on +// the way into the file has letters to move. +const ejectedPreset = "filename-handling" + +// The file -o writes is byte for byte what eject would have printed, and +// nothing goes to standard output. +func TestEjectWritesTheFileByteForByteWhatItWouldPrint(t *testing.T) { + code, printed, errOut := run(t, "preset", "eject", ejectedPreset) + if code != cli.ExitOK || !bytes.ContainsFunc([]byte(printed), func(r rune) bool { return r > 0x7f }) { + t.Fatalf("eject to standard output ended %d with %d bytes, none outside ASCII, so this guard compares nothing worth comparing: %s", + code, len(printed), errOut) + } + path := filepath.Join(t.TempDir(), "my.yaml") + code, out, errOut := run(t, "preset", "eject", ejectedPreset, "-o", path) + if code != cli.ExitOK { + t.Fatalf("eject -o ended %d: %s", code, errOut) + } + if out != "" { + t.Errorf("eject -o put %d bytes on standard output as well as into the file", len(out)) + } + if !strings.Contains(errOut, "recipe: "+path) { + t.Errorf("eject -o does not say where the recipe went:\n%s", errOut) + } + written, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(written, []byte(printed)) { + t.Errorf("the file holds %d bytes and eject prints %d - they differ, so the file is not the recipe PR5 promises", + len(written), len(printed)) + } +} + +// A file already at the name is refused and left exactly as it was - it may +// be a recipe somebody ejected and then edited. Nothing else is left behind, +// and a directory that is not there is refused the same way. +func TestEjectLeavesAFileAlreadyThereAsItIs(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "my.yaml") + edited := []byte("# somebody's edits\n") + if err := os.WriteFile(path, edited, 0o644); err != nil { + t.Fatal(err) + } + code, _, errOut := run(t, "preset", "eject", ejectedPreset, "-o", path) + if code != cli.ExitIO { + t.Errorf("eject -o over a file ended %d rather than %d: %s", code, cli.ExitIO, errOut) + } + if got, err := os.ReadFile(path); err != nil || !bytes.Equal(got, edited) { + t.Errorf("eject -o changed a file that was already there: %q, %v", got, err) + } + if left := namesIn(t, dir); len(left) != 1 { + t.Errorf("the refused eject left %v in the directory", left) + } + + missing := filepath.Join(dir, "not-there", "my.yaml") + if code, _, errOut := run(t, "preset", "eject", ejectedPreset, "-o", missing); code != cli.ExitIO { + t.Errorf("eject -o into a directory that is not there ended %d rather than %d: %s", code, cli.ExitIO, errOut) + } +} + +// A write that fails after the name was claimed takes the claim back, so no +// empty my.yaml is left for somebody to run and wonder at. +func TestEjectThatCannotWriteLeavesNoEmptyFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "my.yaml") + // A directory where the recipe is written first, under its temporary + // name, so the write fails after the claim and before the rename. + if err := os.MkdirAll(core.SiblingPath(path, core.WritingMarker), 0o755); err != nil { + t.Fatal(err) + } + code, _, errOut := run(t, "preset", "eject", ejectedPreset, "-o", path) + if code != cli.ExitIO { + t.Errorf("an eject that could not write ended %d rather than %d: %s", code, cli.ExitIO, errOut) + } + if _, err := os.Lstat(path); err == nil { + t.Error("the eject that could not write left the claimed name behind as an empty file") + } +} + +// An empty name and "-" are refused as usage, and nothing is written - "-" is +// not standard output here, leaving -o out is. +func TestEjectRefusesAFileNameThatIsNotOne(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + for _, name := range []string{"", "-"} { + code, out, errOut := run(t, "preset", "eject", ejectedPreset, "-o", name) + if code != cli.ExitUsage || out != "" { + t.Errorf("eject -o %q ended %d with %d bytes on standard output: %s", name, code, len(out), errOut) + } + } + if left := namesIn(t, dir); len(left) != 0 { + t.Errorf("a refused eject wrote %v", left) + } +} + +// A recipe in UTF-16 is refused, in either byte order, and the refusal says +// where such a file comes from and what to do instead. The general refusal of +// a file that is not UTF-8 says none of that. +func TestARecipeSavedAsUTF16IsRefusedAndSaysWhere(t *testing.T) { + _, printed, _ := run(t, "preset", "eject", ejectedPreset) + units := utf16.Encode([]rune(printed)) + for _, order := range []struct { + name string + mark []byte + as binary.AppendByteOrder + }{ + {"little endian, as PowerShell 5.1 writes it", []byte{0xff, 0xfe}, binary.LittleEndian}, + {"big endian", []byte{0xfe, 0xff}, binary.BigEndian}, + } { + body := append([]byte{}, order.mark...) + for _, u := range units { + body = order.as.AppendUint16(body, u) + } + path := filepath.Join(t.TempDir(), "my.yaml") + if err := os.WriteFile(path, body, 0o644); err != nil { + t.Fatal(err) + } + code, _, errOut := run(t, "validate", path) + if code != cli.ExitRecipe { + t.Errorf("%s: a UTF-16 recipe ended %d rather than %d", order.name, code, cli.ExitRecipe) + } + for _, want := range []string{"UTF-16", "PowerShell 5.1", "-o my.yaml"} { + if !strings.Contains(errOut, want) { + t.Errorf("%s: the refusal does not say %q:\n%s", order.name, want, errOut) + } + } + } + + cp1250 := filepath.Join(t.TempDir(), "r.yaml") + body := append([]byte("version: 1\ntargets:\n - id: t\n format: txt\n size: 1kb\n name: za"), 0xbf, 0xf3, 0xb3, 0xe6) + if err := os.WriteFile(cp1250, append(body, ".txt\n"...), 0o644); err != nil { + t.Fatal(err) + } + if _, _, errOut := run(t, "validate", cp1250); strings.Contains(errOut, "UTF-16") { + t.Errorf("a cp1250 recipe is refused as UTF-16:\n%s", errOut) + } +} diff --git a/internal/recipe/canonical.go b/internal/recipe/canonical.go index a3da2491..0d1b8803 100644 --- a/internal/recipe/canonical.go +++ b/internal/recipe/canonical.go @@ -178,6 +178,17 @@ func withoutBOM(src []byte) []byte { return []byte(out) } +// isUTF16 says a file starts with the byte order mark of UTF-16, in either +// order of bytes. Neither pair can begin a UTF-8 file, so this is asked only of +// a file already found not to be UTF-8. UTF-32 in little endian order starts +// with the same two bytes and is named UTF-16 too, which says the right thing: +// not UTF-8, and why. UTF-16 without a mark has no sign to read and gets the +// general sentence. +func isUTF16(src []byte) bool { + s := string(src) + return strings.HasPrefix(s, "\xff\xfe") || strings.HasPrefix(s, "\xfe\xff") +} + // recipesIn counts the documents in a parsed file that carry a recipe. // // It is deliberately not len(f.Docs), and the difference is not academic. diff --git a/internal/recipe/errors.go b/internal/recipe/errors.go index f0069396..6b6813bf 100644 --- a/internal/recipe/errors.go +++ b/internal/recipe/errors.go @@ -116,6 +116,10 @@ func (e *ValidationError) Error() string { type SyntaxError struct { Name string Detail string + // UTF16 says the file starts with the byte order mark of UTF-16. The + // sentence above says what the file is and why. What to do instead + // depends on the surface that read it, so the command line adds that. + UTF16 bool } func (e *SyntaxError) Error() string { diff --git a/internal/recipe/recipe.go b/internal/recipe/recipe.go index 16377be6..dcecc4b9 100644 --- a/internal/recipe/recipe.go +++ b/internal/recipe/recipe.go @@ -213,6 +213,17 @@ func decode(src []byte, name string) (rawRecipe, error) { // one step earlier: what somebody typed is what they get, or they are told // why not. if !utf8.Valid(src) { + // UTF-16 has its own sentence, because the usual way to get one is not + // saving at all. Windows PowerShell 5.1 writes UTF-16 for "> my.yaml", + // so "save the file as UTF-8" told somebody to do something they never + // did (O245, measured 2026-09-25). Read as UTF-16 it is not, either: on + // the way into that file PowerShell decoded the output with the + // console's code page, and on a stock console that has already changed + // every letter outside ASCII - reading it would turn this loud refusal + // into names quietly different from the ones asked for. + if isUTF16(src) { + return raw, &SyntaxError{Name: name, UTF16: true, Detail: "this file is UTF-16, and a recipe is read as UTF-8. Windows PowerShell 5.1 writes UTF-16 whenever the output of a command is redirected into a file with >, and on the way it may already have changed every letter outside ASCII, so the file is refused rather than read"} + } return raw, &SyntaxError{Name: name, Detail: "this file is not valid UTF-8. Every character that could not be read would come back as a replacement mark, so a name written with accents would produce a file called something else. Save the file as UTF-8 and try again"} }