Skip to content

Commit bda1961

Browse files
committed
engine, core: the engine's refusals arrive in three parts too
The last layer that still handed a report one sentence. Seventeen messages behind one type, so unlike the format and preset refusals they cannot each assemble their own - there is one join rule for all of them, and that is why this was a decision rather than a mechanical change. The rule is how these refusals have always read: what is wrong, a dash, why the rule is there, a full stop, what to do instead. A message that has not been cut into parts carries all of it in Detail and comes out exactly as before, so there is no half finished middle - the same sentence written in one piece or in three. Measured against the previous binary, thirty nine refusals and every stored screen. Three fold back byte for byte: the ceiling on files, the output directory that is a file, and the name template. Two were left alone because they have nothing to separate. Twelve change, all the same way and nothing gained or lost - a full stop or a colon between what and why becomes a dash, the dash before what to do becomes a full stop, and the remedy takes a capital letter: before: ... which holds the character "<". Windows refuses that character ... one thing everywhere - take the character out, or ask for ... after: ... which holds the character "<" - Windows refuses that character ... one thing everywhere. Take the character out, or ask for ... Shown to the owner as the diff of every changed sentence before any of it was committed, and accepted there. One stored screen moved with it, generate-empty, by one line: the refusal about asking for zero files. The picture was looked at rather than regenerated and waved through - the message sits under its box, the box carries its red edge, and nothing else on the screen shifted. core.ErrTooManyFiles is now built from the two parts rather than beside them, so the sentence the command line prints for --count and the parts the report carries cannot come to disagree. Its other caller is untouched and says what it always said.
1 parent d65f1d1 commit bda1961

6 files changed

Lines changed: 110 additions & 42 deletions

File tree

‎internal/core/limits.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,16 @@ const MaxFilesPerRun = 1_000_000
3939
//
4040
// It carries the four parts every refusal in this tool carries, so both callers
4141
// report it in the same words rather than each phrasing it again.
42-
var ErrTooManyFiles = errors.New(
43-
"this build plans at most 1000000 files in one run, because the whole plan is worked out in memory before anything is written - " +
44-
"that is what lets a run that cannot succeed be refused before the first byte. " +
45-
"Ask for fewer files, or split the work into several runs")
42+
var ErrTooManyFiles = errors.New(TooManyFilesWhy + ". " + TooManyFilesFix)
43+
44+
// The same refusal in the two parts a report keeps apart. ErrTooManyFiles is
45+
// built from them rather than beside them, so the sentence and the parts cannot
46+
// come to disagree - the compiler is the proof.
47+
const (
48+
TooManyFilesWhy = "this build plans at most 1000000 files in one run, because the whole plan is worked out in memory before anything is written - " +
49+
"that is what lets a run that cannot succeed be refused before the first byte"
50+
TooManyFilesFix = "Ask for fewer files, or split the work into several runs"
51+
)
4652

4753
// PartialMarker is what a file being written is called before it is finished.
4854
//

‎internal/engine/engine.go‎

Lines changed: 51 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -210,15 +210,21 @@ type PlannedFile struct {
210210
// the run rather than about one entry.
211211
func settleTarget(t *Target, opt Options, seen map[string]bool) (format.Descriptor, error) {
212212
if t.ID == "" {
213-
return format.Descriptor{}, &RecipeError{Setting: SettingID, Detail: "a target has no id: every target needs a stable id, it anchors the seed and links to the manifest"}
213+
return format.Descriptor{}, &RecipeError{Setting: SettingID,
214+
Detail: "a target has no id",
215+
Because: "every target needs a stable id, it anchors the seed and links to the manifest"}
214216
}
215217
if seen[t.ID] {
216-
return format.Descriptor{}, &RecipeError{Setting: SettingID, Detail: fmt.Sprintf("target id %q is used twice: ids identify targets, so a duplicate is an error rather than a silent overwrite", t.ID)}
218+
return format.Descriptor{}, &RecipeError{Setting: SettingID,
219+
Detail: fmt.Sprintf("target id %q is used twice", t.ID),
220+
Because: "ids identify targets, so a duplicate is an error rather than a silent overwrite"}
217221
}
218222
seen[t.ID] = true
219223

220224
if len(t.Sizes) == 0 {
221-
return format.Descriptor{}, &RecipeError{Setting: SettingCount, Detail: fmt.Sprintf("target %q asks for 0 files: ask for at least one", t.ID)}
225+
return format.Descriptor{}, &RecipeError{Setting: SettingCount,
226+
Detail: fmt.Sprintf("target %q asks for 0 files", t.ID),
227+
Remedy: "Ask for at least one"}
222228
}
223229

224230
desc, err := format.Get(t.Format)
@@ -297,7 +303,9 @@ func Plan(targets []Target, opt Options) ([]PlannedFile, error) {
297303
}
298304
}
299305
if opt.OutDir == "" {
300-
return nil, &RecipeError{Setting: SettingOutDir, Detail: "the output directory is empty: name a directory, for example ./fixtures, or leave it out to use the current one"}
306+
return nil, &RecipeError{Setting: SettingOutDir,
307+
Detail: "the output directory is empty",
308+
Remedy: "Name a directory, for example ./fixtures, or leave it out to use the current one"}
301309
}
302310

303311
for i := range targets {
@@ -328,10 +336,11 @@ func Plan(targets []Target, opt Options) ([]PlannedFile, error) {
328336
// this line "validate --json" carried no "at" for it.
329337
return nil, &RecipeError{
330338
Setting: core.TargetAddress(i+1, SettingCount),
331-
Detail: fmt.Sprintf(
332-
"this run asks for %s across %s - %s",
339+
Detail: fmt.Sprintf("this run asks for %s across %s",
333340
core.Count(totalFiles, "file", "files"),
334-
core.Count(len(targets), "target", "targets"), core.ErrTooManyFiles)}
341+
core.Count(len(targets), "target", "targets")),
342+
Because: core.TooManyFilesWhy,
343+
Remedy: core.TooManyFilesFix}
335344
}
336345

337346
for idx, size := range t.Sizes {
@@ -394,8 +403,9 @@ func Plan(targets []Target, opt Options) ([]PlannedFile, error) {
394403
// cannot place either way.
395404
key := collisionKey(name)
396405
if owner, clash := names[key]; clash {
397-
return nil, &RecipeError{Detail: collisionDetail(owner, t.ID, name) +
398-
" - give one of them a name template containing " + indexToken}
406+
return nil, &RecipeError{
407+
Detail: collisionDetail(owner, t.ID, name),
408+
Remedy: "Give one of them a name template containing " + indexToken}
399409
}
400410
names[key] = nameOwner{id: t.ID, name: name}
401411

@@ -405,8 +415,8 @@ func Plan(targets []Target, opt Options) ([]PlannedFile, error) {
405415
// change belongs to a target.
406416
return nil, &RecipeError{
407417
Setting: core.TargetAddress(i+1, format.SettingSize),
408-
Detail: fmt.Sprintf(
409-
"target %q brings the run to a size that is too large to measure: %s", t.ID, err)}
418+
Detail: fmt.Sprintf("target %q brings the run to a size that is too large to measure", t.ID),
419+
Because: err.Error()}
410420
}
411421

412422
out = append(out, PlannedFile{
@@ -471,9 +481,9 @@ func preflight(files []PlannedFile, opt Options) error {
471481
// something at it. The system reports ENOTDIR and our mapping only knew
472482
// "missing", "no permission" and "already there".
473483
if info, err := os.Stat(opt.OutDir); err == nil && !info.IsDir() {
474-
return &RecipeError{Setting: SettingOutDir, Detail: fmt.Sprintf(
475-
"the output directory %s is a file, not a directory. Point the output directory at a directory, or at one that does not exist yet and it will be created",
476-
opt.OutDir)}
484+
return &RecipeError{Setting: SettingOutDir,
485+
Detail: fmt.Sprintf("the output directory %s is a file, not a directory", opt.OutDir),
486+
Remedy: "Point the output directory at a directory, or at one that does not exist yet and it will be created"}
477487
}
478488

479489
// The manifest is checked with the files it would describe, and leaving it
@@ -903,9 +913,9 @@ func renderName(t *Target, d format.Descriptor, index int) (string, error) {
903913
// somebody ends up with a file called invoice_{index}.pdf rather than the
904914
// numbering they asked for.
905915
if strings.Contains(name, "{") {
906-
return "", &RecipeError{Setting: SettingName, Detail: fmt.Sprintf(
907-
"target %q has a name template this build does not understand: %q. The only placeholder is %s, so a name looks like invoice_%s.pdf",
908-
t.ID, tmpl, indexToken, indexToken)}
916+
return "", &RecipeError{Setting: SettingName,
917+
Detail: fmt.Sprintf("target %q has a name template this build does not understand: %q", t.ID, tmpl),
918+
Remedy: fmt.Sprintf("The only placeholder is %s, so a name looks like invoice_%s.pdf", indexToken, indexToken)}
909919
}
910920

911921
if err := checkFileName(SettingName, fmt.Sprintf("target %q", t.ID), name); err != nil {
@@ -931,9 +941,10 @@ func checkFileName(setting, where, name string) error {
931941
return &RecipeError{Setting: setting, Detail: fmt.Sprintf("%s produces a file with no name", where)}
932942

933943
case strings.ContainsAny(name, `/\`):
934-
return &RecipeError{Setting: setting, Detail: fmt.Sprintf(
935-
"%s produces the name %q, which is a path rather than a file name. Names stay inside the output directory, and a separator is refused on every system so that a recipe works everywhere. Choose the directory with the output setting instead",
936-
where, name)}
944+
return &RecipeError{Setting: setting,
945+
Detail: fmt.Sprintf("%s produces the name %q, which is a path rather than a file name", where, name),
946+
Because: "names stay inside the output directory, and a separator is refused on every system so that a recipe works everywhere",
947+
Remedy: "Choose the directory with the output setting instead"}
937948

938949
// A colon, on every system, for the same reason as a separator.
939950
//
@@ -950,9 +961,10 @@ func checkFileName(setting, where, name string) error {
950961
// travels between machines by design, and one that quietly leaves debris on
951962
// somebody else's is worse than one refused on all of them.
952963
case strings.Contains(name, ":"):
953-
return &RecipeError{Setting: setting, Detail: fmt.Sprintf(
954-
"%s produces the name %q, which holds a colon. Windows reads that as a drive or as an alternate data stream rather than as part of the name, so the file arrives called something else or not at all. It is refused on every system so that a recipe means one thing everywhere - take the colon out, or ask for the file inside an archive where the name survives",
955-
where, name)}
964+
return &RecipeError{Setting: setting,
965+
Detail: fmt.Sprintf("%s produces the name %q, which holds a colon", where, name),
966+
Because: "Windows reads that as a drive or as an alternate data stream rather than as part of the name, so the file arrives called something else or not at all. It is refused on every system so that a recipe means one thing everywhere",
967+
Remedy: "Take the colon out, or ask for the file inside an archive where the name survives"}
956968

957969
// Characters Windows will not put in a file name, refused on every system
958970
// for the same reason as the separator and the colon above.
@@ -974,9 +986,10 @@ func checkFileName(setting, where, name string) error {
974986
// the host filesystem. See D10.
975987
case firstForbidden(name) != 0:
976988
bad := firstForbidden(name)
977-
return &RecipeError{Setting: setting, Detail: fmt.Sprintf(
978-
"%s produces the name %q, which holds %s. Windows refuses that character in a file name, so the file is not written there at all. It is refused on every system so that a recipe means one thing everywhere - take the character out, or ask for the file inside an archive where the name survives",
979-
where, name, describeForbidden(bad))}
989+
return &RecipeError{Setting: setting,
990+
Detail: fmt.Sprintf("%s produces the name %q, which holds %s", where, name, describeForbidden(bad)),
991+
Because: "Windows refuses that character in a file name, so the file is not written there at all. It is refused on every system so that a recipe means one thing everywhere",
992+
Remedy: "Take the character out, or ask for the file inside an archive where the name survives"}
980993

981994
// One reserved device name, and one only.
982995
//
@@ -999,9 +1012,10 @@ func checkFileName(setting, where, name string) error {
9991012
// on both editions - so this is not the "any extension" rule the folklore
10001013
// describes either.
10011014
case strings.EqualFold(name, "nul"):
1002-
return &RecipeError{Setting: setting, Detail: fmt.Sprintf(
1003-
"%s produces the name %q, which names the null device on Windows rather than a file. Writing there succeeds and the bytes go nowhere, so the run would record a file that is not on the disk. It is refused on every system so that a recipe means one thing everywhere - give it an extension, nul.txt is an ordinary name, or choose another one",
1004-
where, name)}
1015+
return &RecipeError{Setting: setting,
1016+
Detail: fmt.Sprintf("%s produces the name %q, which names the null device on Windows rather than a file", where, name),
1017+
Because: "writing there succeeds and the bytes go nowhere, so the run would record a file that is not on the disk. It is refused on every system so that a recipe means one thing everywhere",
1018+
Remedy: "Give it an extension, nul.txt is an ordinary name, or choose another one"}
10051019

10061020
case name == "." || name == "..":
10071021
return &RecipeError{Setting: setting, Detail: fmt.Sprintf(
@@ -1021,9 +1035,10 @@ func checkFileName(setting, where, name string) error {
10211035
// the name laboratory, which writes it into an archive rather than onto the
10221036
// host filesystem for exactly this reason. See D10.
10231037
case strings.HasSuffix(name, ".") || strings.HasSuffix(name, " "):
1024-
return &RecipeError{Setting: setting, Detail: fmt.Sprintf(
1025-
"%s produces the name %q, which ends in a dot or a space. Windows stores such a name without it, so the file on disk would not be the file the manifest describes and verify would report both. Take the last character off, or ask for the file inside an archive where the name survives",
1026-
where, name)}
1038+
return &RecipeError{Setting: setting,
1039+
Detail: fmt.Sprintf("%s produces the name %q, which ends in a dot or a space", where, name),
1040+
Because: "Windows stores such a name without it, so the file on disk would not be the file the manifest describes and verify would report both",
1041+
Remedy: "Take the last character off, or ask for the file inside an archive where the name survives"}
10271042

10281043
// Judged the same way on every system, like the separator above. Using
10291044
// filepath here asks the machine this build runs on, and the answer
@@ -1032,9 +1047,10 @@ func checkFileName(setting, where, name string) error {
10321047
// machine failed on the next. That is the failure this rule exists to
10331048
// prevent, arriving through the rule itself.
10341049
case filepath.IsAbs(name) || core.HasVolumeName(name):
1035-
return &RecipeError{Setting: setting, Detail: fmt.Sprintf(
1036-
"%s produces the absolute path %q. A recipe carries no absolute paths, because then it only works on the machine it was written on. Choose the directory with the output setting instead",
1037-
where, name)}
1050+
return &RecipeError{Setting: setting,
1051+
Detail: fmt.Sprintf("%s produces the absolute path %q", where, name),
1052+
Because: "a recipe carries no absolute paths, because then it only works on the machine it was written on",
1053+
Remedy: "Choose the directory with the output setting instead"}
10381054
}
10391055
return nil
10401056
}

‎internal/engine/errors.go‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,15 @@ import (
1818
// RecipeError is a request that is well formed but asks for something that
1919
// makes no sense.
2020
type RecipeError struct {
21-
Detail string
21+
// Detail is what is wrong. Because is why the rule exists and Remedy is
22+
// what to do instead - the other two of the three parts every refusal in
23+
// this tool has, kept apart so a report can carry them apart and a script
24+
// reading it does not have to take a sentence written for a person back to
25+
// pieces. Both may be empty, and then the whole refusal is in Detail and
26+
// reads exactly as it did before either field existed.
27+
Detail string
28+
Because string
29+
Remedy string
2230

2331
// Setting is which setting the refusal is about, where it is about one.
2432
//
@@ -45,11 +53,44 @@ func (e *RecipeError) Error() string {
4553

4654
// InTheWordsOf is this refusal with the setting named the way one surface names
4755
// it - see core.SettingSlot. Error is this with the recipe key.
56+
//
57+
// One sentence assembled from the parts, joined the way these refusals have
58+
// always read: what is wrong, a dash, why the rule is there, a full stop, what
59+
// to do instead. A refusal that has not been cut into parts yet carries all of
60+
// it in Detail and comes out exactly as it did before, so the two states are
61+
// not a half finished middle - they are the same sentence written in one piece
62+
// or in three.
4863
func (e *RecipeError) InTheWordsOf(name string) string {
4964
if name == "" {
5065
name = core.LastSettingSegment(e.Setting)
5166
}
52-
return core.InTheWordsOf(e.Detail, name)
67+
message := e.Detail
68+
if e.Because != "" {
69+
message += " - " + e.Because
70+
}
71+
if e.Remedy != "" {
72+
message += ". " + e.Remedy
73+
}
74+
return core.InTheWordsOf(message, name)
75+
}
76+
77+
// The three parts a report keeps apart, answering the same names the format and
78+
// preset refusals answer so that nothing above has to know this type.
79+
//
80+
// The fields are Because and Remedy rather than Why and Fix because a field
81+
// cannot share a name with a method, and the method names are the ones already
82+
// spoken here - UnknownPropertyError has answered to them since the recipe
83+
// reader started reporting four parts.
84+
func (e *RecipeError) What() string {
85+
return core.InTheWordsOf(e.Detail, core.LastSettingSegment(e.Setting))
86+
}
87+
88+
func (e *RecipeError) Why() string {
89+
return core.InTheWordsOf(e.Because, core.LastSettingSegment(e.Setting))
90+
}
91+
92+
func (e *RecipeError) Instead() string {
93+
return core.InTheWordsOf(e.Remedy, core.LastSettingSegment(e.Setting))
5394
}
5495

5596
// AboutSetting lets a window place this message without knowing this type.

‎internal/guard/refusaladdress_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,11 @@ func TestARefusalFromBelowTheReaderArrivesInItsThreeParts(t *testing.T) {
631631
{"a size the format cannot deliver", "version: 1\ntargets:\n - id: a\n format: pdf\n size: 10\n"},
632632
{"contains asked of a format that holds nothing", "version: 1\ntargets:\n - id: a\n format: txt\n size: 1kb\n contains:\n - format: txt\n count: 2\n size: 100\n"},
633633
{"a value a format setting will not take", "version: 1\ntargets:\n - id: a\n format: bmp\n size: 1mb\n properties:\n width: \"99999\"\n"},
634+
// And the engine's own, which is one type behind seventeen messages -
635+
// so these parts come from one join rule rather than a sentence each,
636+
// and the join has to leave the sentence reading as it always did.
637+
{"a name the host cannot store", "version: 1\ntargets:\n - id: a\n format: txt\n size: 1kb\n name: \"a<b.txt\"\n"},
638+
{"more files than the run may plan", "version: 1\ntargets:\n - id: a\n format: txt\n count: 1\n size: 1kb\n - id: b\n format: txt\n count: 1000000\n size: 1kb\n"},
634639
}
635640

636641
for _, c := range cases {
119 Bytes
Loading

‎internal/guard/testdata/screens/generate-empty.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@
152152
<container pos="0,65" size="808x31">
153153
<widget size="808x31" type="*widget.Label">
154154
<widget size="808x31" type="*widget.RichText">
155-
<text color="error" pos="6,6" size="310x19">target "files" asks for 0 files: ask for at least one</text>
155+
<text color="error" pos="6,6" size="311x19">target "files" asks for 0 files. Ask for at least one</text>
156156
</widget>
157157
</widget>
158158
</container>

0 commit comments

Comments
 (0)