Skip to content

Commit 5643e6c

Browse files
committed
Name several To-dos tools without a doubled s, and make every container spell its plural
A cross-project to-do list move into a project with two To-dos tools refused with "Dest has 2 To-doss that match": the To-dos container set no plural, so the message appended "s" to "To-dos". The container came in with todo-moves (#976); it is fixed here, at the top of the stack, rather than on that branch. The container now says "To-dos tools". The fallback that appended "s" is gone: every filingContainer spells its plural out, and a test reads the package's source and fails on any filingContainer literal without one, so the next container cannot fall into the same message.
1 parent 409b8ec commit 5643e6c

7 files changed

Lines changed: 54 additions & 11 deletions

File tree

‎internal/commands/cards.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2712,6 +2712,7 @@ and On hold. The command waits for Basecamp to make the copy and shows it.`,
27122712
var cardTableContainer = filingContainer{
27132713
dockName: "kanban_board",
27142714
noun: "card table",
2715+
plural: "card tables",
27152716
flag: "--to-card-table",
27162717
urlKind: "card_tables",
27172718
urlShape: "…/card_tables/<id>",

‎internal/commands/filing_family.go‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ type filingContainer struct {
2626
// dockName is the tool's name in a project's dock: "message_board".
2727
dockName string
2828
noun string
29-
// plural is the noun's plural when appending "s" is wrong.
29+
// plural names several of them in messages: "message boards", or
30+
// "To-dos tools" where appending "s" reads wrong. Every container sets it.
3031
plural string
3132
flag string
3233
urlKind string
@@ -35,13 +36,6 @@ type filingContainer struct {
3536
get func(ctx context.Context, app *appctx.App, id int64) (filingTarget, error)
3637
}
3738

38-
func (c filingContainer) plurals() string {
39-
if c.plural != "" {
40-
return c.plural
41-
}
42-
return output.PluralNoun(c.noun)
43-
}
44-
4539
// filingTarget is the container a recording is moved or copied into.
4640
type filingTarget struct {
4741
id int64
@@ -148,7 +142,7 @@ func resolveFilingTarget(cmd *cobra.Command, app *appctx.App, c filingContainer,
148142
}
149143
if url != "" {
150144
return filingTarget{}, filingURLNotFound(c.flag, c.noun, url, projectName,
151-
fmt.Sprintf("Its %s: %s", c.plurals(), formatDockTools(enabled)))
145+
fmt.Sprintf("Its %s: %s", c.plural, formatDockTools(enabled)))
152146
}
153147
switch len(named) {
154148
case 0:
@@ -162,7 +156,7 @@ func resolveFilingTarget(cmd *cobra.Command, app *appctx.App, c filingContainer,
162156
}
163157
return filingTarget{}, output.ErrUsageHint(
164158
fmt.Sprintf("%s %q is not a %s in %s", c.flag, toContainer, c.noun, projectName),
165-
fmt.Sprintf("Its %s: %s", c.plurals(), formatDockTools(enabled)),
159+
fmt.Sprintf("Its %s: %s", c.plural, formatDockTools(enabled)),
166160
)
167161
}
168162

@@ -214,7 +208,7 @@ func ambiguousFilingContainerError(c filingContainer, projectName string, tools
214208
}
215209
return &output.Error{
216210
Code: output.CodeAmbiguous,
217-
Message: fmt.Sprintf("%s has %d %s that match", projectName, len(tools), c.plurals()),
211+
Message: fmt.Sprintf("%s has %d %s that match", projectName, len(tools), c.plural),
218212
Hint: fmt.Sprintf("Specify one with %s <id>:\n%s", c.flag, strings.Join(lines, "\n")),
219213
}
220214
}

‎internal/commands/filing_family_test.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,13 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"go/ast"
8+
"go/parser"
9+
"go/token"
710
"io"
811
"net/http"
12+
"os"
13+
"path/filepath"
914
"regexp"
1015
"slices"
1116
"strconv"
@@ -142,6 +147,7 @@ func boardsContainer() filingContainer {
142147
return filingContainer{
143148
dockName: "message_board",
144149
noun: "message board",
150+
plural: "message boards",
145151
flag: "--to-board",
146152
urlKind: "message_boards",
147153
urlShape: "…/message_boards/<id>",
@@ -164,6 +170,44 @@ func resolveTestTarget(t *testing.T, transport *familyTransport, toProject, toCo
164170
return target, err
165171
}
166172

173+
// Appending "s" to a container's noun once named a project's To-dos tools
174+
// "To-doss", so every filingContainer literal spells its plural out. The test
175+
// reads the package's source, so it covers containers no test resolves.
176+
func TestEveryFilingContainerNamesItsPlural(t *testing.T) {
177+
files, err := filepath.Glob("*.go")
178+
require.NoError(t, err)
179+
fset := token.NewFileSet()
180+
var containers int
181+
for _, name := range files {
182+
src, err := os.ReadFile(name)
183+
require.NoError(t, err)
184+
file, err := parser.ParseFile(fset, name, src, 0)
185+
require.NoError(t, err)
186+
ast.Inspect(file, func(n ast.Node) bool {
187+
lit, ok := n.(*ast.CompositeLit)
188+
if !ok {
189+
return true
190+
}
191+
if typ, ok := lit.Type.(*ast.Ident); !ok || typ.Name != "filingContainer" {
192+
return true
193+
}
194+
containers++
195+
named := slices.ContainsFunc(lit.Elts, func(e ast.Expr) bool {
196+
kv, ok := e.(*ast.KeyValueExpr)
197+
if !ok {
198+
return false
199+
}
200+
key, ok := kv.Key.(*ast.Ident)
201+
value, isString := kv.Value.(*ast.BasicLit)
202+
return ok && key.Name == "plural" && isString && value.Value != `""`
203+
})
204+
assert.True(t, named, "%s: a filingContainer sets plural", fset.Position(lit.Pos()))
205+
return true
206+
})
207+
}
208+
assert.GreaterOrEqual(t, containers, 7, "the scan finds every family's container")
209+
}
210+
167211
func dockTransport(dock string) *familyTransport {
168212
return &familyTransport{gets: map[string]string{
169213
"/projects": filingProjects,

‎internal/commands/messages.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,7 @@ func messageFamily() filingFamily {
11211121
into: filingContainer{
11221122
dockName: "message_board",
11231123
noun: "message board",
1124+
plural: "message boards",
11241125
flag: "--to-board",
11251126
urlKind: "message_boards",
11261127
urlShape: "…/message_boards/<id>",

‎internal/commands/schedule.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -987,6 +987,7 @@ func scheduleEntryFamily() filingFamily {
987987
into: filingContainer{
988988
dockName: "schedule",
989989
noun: "schedule",
990+
plural: "schedules",
990991
flag: "--to-schedule",
991992
urlKind: "schedules",
992993
urlShape: "…/schedules/<id>",

‎internal/commands/todolists.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,7 @@ func readFiledTodolist(app *appctx.App, sourceID int64) func(context.Context, in
11831183
var todosetContainer = filingContainer{
11841184
dockName: "todoset",
11851185
noun: "To-dos",
1186+
plural: "To-dos tools",
11861187
flag: "--to-todoset",
11871188
urlKind: "todosets",
11881189
urlShape: "…/todosets/<id>",

‎internal/commands/todolists_filing_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ func TestTodolistsMoveToAProjectWithSeveralTodosetsIsAmbiguous(t *testing.T) {
3232
e := requireUsageRefusal(t, run, transport)
3333

3434
assert.Equal(t, output.CodeAmbiguous, e.Code)
35+
assert.Equal(t, "Dest has 2 To-dos tools that match", e.Message)
3536
assert.Contains(t, e.Hint, "--to-todoset")
3637
assert.Contains(t, e.Hint, "900 To-dos")
3738
assert.Contains(t, e.Hint, "950 To-dos")

0 commit comments

Comments
 (0)