diff --git a/cmd/things/add.go b/cmd/things/add.go index 8ed389a..34c3c8b 100644 --- a/cmd/things/add.go +++ b/cmd/things/add.go @@ -45,7 +45,7 @@ func (c *AddCmd) Run(d *Deps) error { return err } // Things matches list by title only; a uuid has to go as list-id. - target, dest := resolveAddTarget(d, list, c.Heading) + target, dest := resolveAddTarget(d, ¶ms.List, ¶ms.Heading) if target.ByUUID { params.List, params.ListID = "", target.UUID } @@ -82,8 +82,11 @@ func preAdd(d *Deps, kind string, p addParams, flags TagFlags) error { // the Inbox or leaves out the heading without reporting it. The add still // goes ahead. dest is where Things will file it, for the read-back; it checks // nothing when the database cannot be read. A database that cannot be read -// gives no warning here: the tag check or the read-back reports that. -func resolveAddTarget(d *Deps, list, heading string) (db.Target, createdDest) { +// gives no warning here: the tag check or the read-back reports that. It +// sets list and heading to the names to send, trimmed when only that form +// matches (addTargetName). +func resolveAddTarget(d *Deps, listp, headingp *string) (db.Target, createdDest) { + list, heading := *listp, *headingp if list == "" { if heading != "" { fmt.Fprintf(d.errOut(), "warning: %s\n", headingNeedsList("--heading", heading, "--list or --project", "put the to-do in the Inbox")) @@ -94,7 +97,7 @@ func resolveAddTarget(d *Deps, list, heading string) (db.Target, createdDest) { if err != nil { return db.Target{}, createdDest{} } - target, headingFound, err := database.AddTarget(list, heading) + target, headingFound, list, heading, err := addTargetName(database, list, heading) switch { case err != nil: return db.Target{}, createdDest{} @@ -104,6 +107,7 @@ func resolveAddTarget(d *Deps, list, heading string) (db.Target, createdDest) { case heading != "" && !headingFound: fmt.Fprintf(d.errOut(), "warning: %s\n", noHeading(listName(list, target), heading, "add the to-do there without a heading")) } + *listp, *headingp = list, heading noteTarget(d, list, "lists", target) return target, addDest(target) } diff --git a/cmd/things/add_test.go b/cmd/things/add_test.go index e7ba616..42ab9fa 100644 --- a/cmd/things/add_test.go +++ b/cmd/things/add_test.go @@ -550,8 +550,9 @@ func TestAddWarnsOnUnresolvedListOrHeading(t *testing.T) { {"unknownHeading", []string{"--list", "Tools", "--heading", "Later"}, `"Tools" has no heading "Later"`, inProj1}, {"headingInArea", []string{"--list", "Personal", "--heading", "Setup"}, `"Personal" has no heading "Setup"`, inArea1}, {"headingWithoutList", []string{"--heading", "Setup"}, `--heading "Setup" needs --list or --project`, ""}, - {"paddedList", []string{"--list", " Tools "}, `no project or area called " Tools "`, ""}, - {"paddedHeading", []string{"--list", "Tools", "--heading", " Setup "}, `"Tools" has no heading " Setup "`, inProj1}, + // Sent trimmed, as only that form matches (addTargetName). + {"paddedList", []string{"--list", " Tools "}, "", inProj1}, + {"paddedHeading", []string{"--list", "Tools", "--heading", " Setup "}, "", underHead1}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -674,7 +675,7 @@ func TestProjectAddWarnsOnUnknownArea(t *testing.T) { {"uuid", "area-1", "", inArea1}, {"paddedUUID", " area-1 ", "", inArea1}, {"unknown", "Nowhere", `no area called "Nowhere"`, ""}, - {"padded", " Personal ", `no area called " Personal "`, ""}, + {"padded", " Personal ", "", inArea1}, {"paddedTitle", "Errands ", "", `area = 'area-2'`}, {"paddedTitleUnpadded", "Errands", `no area called "Errands"`, ""}, {"fullwidth", "Ｐersonal", "", inArea1}, diff --git a/cmd/things/edit.go b/cmd/things/edit.go index 140d29a..8a7f7f1 100644 --- a/cmd/things/edit.go +++ b/cmd/things/edit.go @@ -98,7 +98,8 @@ func (c *EditCmd) checkOwn(d *Deps, database *db.DB, task *model.Task) bool { // checkMove warns when Things will not move the task where the move flags // say, sends a uuid given to --list as list-id, and reports whether the move // flags may change the task. Checked in Things 3 with things:///update: a list -// or heading title is matched ignoring case but not surrounding space, and a +// or heading title is matched ignoring case but not surrounding space, so a +// padded title goes trimmed when only that form matches (addTargetName), and a // uuid only as list-id. A list Things cannot find is ignored: the to-do stays // where it is, or with a heading the heading is looked up as if it came // alone. An unknown heading in a known list moves the to-do to the list with @@ -178,7 +179,14 @@ func (c *EditCmd) checkMove(d *Deps, database *db.DB, task *model.Task) bool { } else { list = *c.List } - t, f, err := database.AddTarget(list, heading) + // A title goes as given or trimmed, whichever Things matches + // (addTargetName). A list-id matches only as a uuid, but a heading + // sent with it is a title all the same. + t, f, sentList, sentHeading, err := addTargetName(database, list, heading) + if err == nil && t.UUID != "" && (t.ByUUID || !byID) { + c.setMoveNames(sentList, sentHeading) + list, heading = sentList, sentHeading + } switch { case err != nil: return true @@ -222,7 +230,7 @@ func (c *EditCmd) checkMove(d *Deps, database *db.DB, task *model.Task) bool { fmt.Fprintf(d.errOut(), "warning: %s\n", headingNeedsList("--heading", heading, "--list or --list-id, as the to-do is not in a project", "leave the to-do where it is")) return false } - t, f, err := database.AddTarget(task.ProjectUUID, heading) + t, f, _, sentHeading, err := addTargetName(database, task.ProjectUUID, heading) switch { case err != nil || t.UUID == "": return true @@ -230,9 +238,22 @@ func (c *EditCmd) checkMove(d *Deps, database *db.DB, task *model.Task) bool { fmt.Fprintf(d.errOut(), "warning: %s\n", noHeading(task.ProjectTitle, heading, "leave the to-do where it is")) return false } + c.setMoveNames("", sentHeading) return task.HeadingUUID != t.Heading } +// setMoveNames sends list and heading, the names checkMove found Things +// matches, in place of --list and --heading as given. An empty one, or a +// flag not given, is left alone. +func (c *EditCmd) setMoveNames(list, heading string) { + if c.List != nil && list != "" { + c.List = &list + } + if c.Heading != nil && heading != "" { + c.Heading = &heading + } +} + // noteListID gives the notes for a move into the list --list-id names // (targetNotes), as checkMove does for a list it resolves, when the id is a // project or area the to-do is not in already. A heading-id Things can find diff --git a/cmd/things/edit_test.go b/cmd/things/edit_test.go index 0f3e46b..212f8a6 100644 --- a/cmd/things/edit_test.go +++ b/cmd/things/edit_test.go @@ -612,7 +612,7 @@ func TestEditWarnsOnUnresolvedMove(t *testing.T) { {"listUUID", []string{"edit", "one-1", "--list", "proj-1"}, "", false}, {"knownHeading", []string{"edit", "one-1", "--list", "Tools", "--heading", "setup"}, "", false}, {"unknownList", []string{"edit", "one-1", "--list", "Nowhere"}, `no project or area called "Nowhere"; the to-do will stay where it is`, true}, - {"paddedList", []string{"edit", "one-1", "--list", " Tools "}, `no project or area called " Tools "`, true}, + {"paddedList", []string{"edit", "one-1", "--list", " Tools "}, "", false}, {"completedProject", []string{"edit", "one-1", "--list", "Old"}, `no project or area called "Old"`, true}, {"unknownListAndTitle", []string{"edit", "one-1", "--list", "Nowhere", "--title", "Post the letter"}, `no project or area called "Nowhere"`, false}, {"unknownListAndHeading", []string{"edit", "one-1", "--list", "Nowhere", "--heading", "Setup"}, `no project or area called "Nowhere"; the to-do will stay where it is`, true}, @@ -637,7 +637,7 @@ func TestEditWarnsOnUnresolvedMove(t *testing.T) { {"unknownHeadingID", []string{"edit", "tool-1", "--heading-id", "nope"}, `no heading with id "nope"; the to-do will stay where it is`, true}, {"unknownHeadingIDWithList", []string{"edit", "one-1", "--list", "Tools", "--heading-id", "nope"}, `no heading with id "nope"; it will ignore --heading-id`, false}, {"unknownHeading", []string{"edit", "one-1", "--list", "Tools", "--heading", "Later"}, `"Tools" has no heading "Later"; Things will move the to-do there without a heading`, false}, - {"paddedHeading", []string{"edit", "one-1", "--list", "Tools", "--heading", " Setup "}, `"Tools" has no heading " Setup "`, false}, + {"paddedHeading", []string{"edit", "one-1", "--list", "Tools", "--heading", " Setup "}, "", false}, {"unknownHeadingInListID", []string{"edit", "one-1", "--list-id", "proj-1", "--heading", "Later"}, `"Tools" has no heading "Later"`, false}, {"knownHeadingInListID", []string{"edit", "one-1", "--list-id", "proj-1", "--heading", "Setup"}, "", false}, {"unknownHeadingInListUUID", []string{"edit", "one-1", "--list", "proj-1", "--heading", "Later"}, `"Tools" has no heading "Later"`, false}, @@ -672,7 +672,7 @@ func TestEditWarnsOnUnresolvedMove(t *testing.T) { {"projectKnownArea", []string{"project", "edit", "repproj-1", "--area", "personal"}, "", false}, {"projectAreaUUID", []string{"project", "edit", "repproj-1", "--area", "area-1"}, "", false}, {"projectUnknownArea", []string{"project", "edit", "repproj-1", "--area", "Nowhere"}, `no area called "Nowhere"; the project will stay where it is`, true}, - {"projectPaddedArea", []string{"project", "edit", "repproj-1", "--area", " Personal "}, `no area called " Personal "`, true}, + {"projectPaddedArea", []string{"project", "edit", "repproj-1", "--area", " Personal "}, "", false}, {"projectSameArea", []string{"project", "edit", "areaproj-1", "--area", "PERSONAL"}, "", true}, {"projectSameAreaFullwidth", []string{"project", "edit", "areaproj-1", "--area", "\uff30ERSONAL"}, "", true}, {"projectSameAreaUUID", []string{"project", "edit", "areaproj-1", "--area", "area-1"}, "", true}, diff --git a/cmd/things/names.go b/cmd/things/names.go new file mode 100644 index 0000000..cbd56a9 --- /dev/null +++ b/cmd/things/names.go @@ -0,0 +1,71 @@ +package main + +import ( + "strings" + + "github.com/ryanlewis/things-cli/internal/db" +) + +// Things matches a list, area or heading title without trimming it, so +// ` Errands ` misses the list called Errands. add, project add and edit +// send a name as given when Things matches it as given, which keeps a title +// that really has surrounding space reachable, and otherwise send it trimmed +// when that matches. A name neither form matches goes as given, and the +// caller warns about it. import does not trim: its payload is Things' own +// format and goes as given. + +// addTargetName is db.AddTarget with that rule applied to list and heading. +// It returns the list and heading to send. An area has no headings, so a +// heading is not retried in one. +func addTargetName(database *db.DB, list, heading string) (target db.Target, headingFound bool, sentList, sentHeading string, err error) { + target, headingFound, err = database.AddTarget(list, heading) + if err != nil { + return db.Target{}, false, list, heading, err + } + sentList, sentHeading = list, heading + if trimmed, ok := trimmedName(list); ok && target.UUID == "" { + t, f, err := database.AddTarget(trimmed, heading) + if err != nil { + return db.Target{}, false, list, heading, err + } + if t.UUID != "" { + target, headingFound, sentList = t, f, trimmed + } + } + if trimmed, ok := trimmedName(heading); ok && target.UUID != "" && !target.Area && !headingFound { + id, err := database.HeadingTarget(target.UUID, trimmed) + if err != nil { + return db.Target{}, false, list, heading, err + } + if id != "" { + target.Heading, headingFound, sentHeading = id, true, trimmed + } + } + return target, headingFound, sentList, sentHeading, nil +} + +// areaTargetName is db.AreaTarget with that rule applied to area. It returns +// the area to send. +func areaTargetName(database *db.DB, area string) (db.Target, string, error) { + t, err := database.AreaTarget(area) + if err != nil { + return db.Target{}, area, err + } + if trimmed, ok := trimmedName(area); ok && t.UUID == "" { + tt, err := database.AreaTarget(trimmed) + if err != nil { + return db.Target{}, area, err + } + if tt.UUID != "" { + return tt, trimmed, nil + } + } + return t, area, nil +} + +// trimmedName is name without surrounding space, and whether that differs +// from name and is still a name. +func trimmedName(name string) (string, bool) { + t := strings.TrimSpace(name) + return t, t != name && t != "" +} diff --git a/cmd/things/names_test.go b/cmd/things/names_test.go new file mode 100644 index 0000000..fa914df --- /dev/null +++ b/cmd/things/names_test.go @@ -0,0 +1,67 @@ +package main + +import ( + "os/exec" + "slices" + "strings" + "testing" + + "github.com/ryanlewis/things-cli/internal/db/dbtest" + "github.com/ryanlewis/things-cli/internal/things" +) + +// A list, area or heading name goes to Things as given when Things matches +// it as given, trimmed when only that form matches, and as given when +// neither does. A title that really has surrounding space keeps it. +func TestTargetNamesSentTrimmedWhenOnlyThatMatches(t *testing.T) { + cases := []struct { + name string + args []string + want []string + }{ + {"addList", []string{"add", "Buy oat milk", "--list", " Tools "}, []string{"list=Tools"}}, + {"addProject", []string{"add", "Buy oat milk", "--project", " Tools ", "--heading", " Setup "}, []string{"list=Tools", "heading=Setup"}}, + {"addArea", []string{"add", "Buy oat milk", "--list", " Personal "}, []string{"list=Personal"}}, + {"addUnknown", []string{"add", "Buy oat milk", "--list", " Nowhere "}, []string{"list=%20Nowhere%20"}}, + {"addUnknownHeading", []string{"add", "Buy oat milk", "--list", "Tools", "--heading", " Later "}, []string{"list=Tools", "heading=%20Later%20"}}, + {"addPaddedTitle", []string{"add", "Buy oat milk", "--list", "Errands "}, []string{"list=Errands%20"}}, + {"projectAdd", []string{"project", "add", "Launch", "--area", " Personal "}, []string{"area=Personal"}}, + {"projectAddPaddedTitle", []string{"project", "add", "Launch", "--area", "Errands "}, []string{"area=Errands%20"}}, + {"edit", []string{"edit", "one-1", "--list", " Tools ", "--heading", " Setup "}, []string{"list=Tools", "heading=Setup"}}, + {"editListIDHeading", []string{"edit", "one-1", "--list-id", "proj-1", "--heading", " Setup "}, []string{"heading=Setup"}}, + {"editOwnHeading", []string{"edit", "tool-1", "--heading", " Setup "}, []string{"heading=Setup"}}, + {"editUnknown", []string{"edit", "one-1", "--list", " Nowhere "}, []string{"list=%20Nowhere%20"}}, + {"projectEdit", []string{"project", "edit", "garden-1", "--area", " Personal "}, []string{"area=Personal"}}, + {"projectEditPaddedTitle", []string{"project", "edit", "garden-1", "--area", "Errands "}, []string{"area=Errands%20"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fastVerify(t) + database, sqlDB := seedWritable(t) + fx := dbtest.NewFixture(t, sqlDB) + fx.Area("area-1", "Personal", 1) + fx.Area("area-2", "Errands ", 2) + fx.Project("proj-1", "Tools", 5) + fx.Project("garden-1", "Garden", 6) + fx.Heading("head-1", "Setup", 1, dbtest.InProject("proj-1")) + fx.Todo("tool-1", "Oil hinges", 8, dbtest.Anytime(), dbtest.InProject("proj-1")) + var url string + prev := things.SetExecCommandForTest(func(_ string, args ...string) *exec.Cmd { + url = args[len(args)-1] + return exec.Command("true") + }) + t.Cleanup(func() { things.SetExecCommandForTest(prev) }) + + _, stderr, _ := runStreams(t, database, append([]string{"--no-verify"}, tc.args...)...) + params := strings.Split(url[strings.Index(url, "?")+1:], "&") + for _, want := range tc.want { + if !slices.Contains(params, want) { + t.Errorf("url = %q, want %s", url, want) + } + } + if strings.Contains(tc.name, "Unknown") != strings.Contains(stderr, "warning: ") { + t.Errorf("stderr = %q, want a warning only for a name neither form matches", stderr) + } + }) + } +} diff --git a/cmd/things/project.go b/cmd/things/project.go index 5d3cf9b..0ca7a98 100644 --- a/cmd/things/project.go +++ b/cmd/things/project.go @@ -44,7 +44,7 @@ func (c *ProjectAddCmd) Run(d *Deps) error { return err } // Things matches area by title only; a uuid has to go as area-id. - target, read := projectArea(d, c.Area, "it will create the project with no area") + target, read := projectArea(d, ¶ms.Area, "it will create the project with no area") // Where Things will file the project, for the read-back: the area the // title or uuid leads to, or none when it leads nowhere. dest := createdDest{checked: true, list: target.UUID} @@ -70,8 +70,11 @@ func (c *ProjectAddCmd) Run(d *Deps) error { // the area and without reporting it: add creates the project with no area, // and update leaves it where it is. fallback says which, for the warning. It // read is false when the database cannot be read, which gives no warning -// here: the read-back reports that. An empty area matches nothing. -func projectArea(d *Deps, area, fallback string) (target db.Target, read bool) { +// here: the read-back reports that. An empty area matches nothing. It sets +// area to the name to send, trimmed when only that form matches +// (areaTargetName). +func projectArea(d *Deps, areap *string, fallback string) (target db.Target, read bool) { + area := *areap if area == "" { return db.Target{}, true } @@ -81,14 +84,17 @@ func projectArea(d *Deps, area, fallback string) (target db.Target, read bool) { } // Not FindAreaUUID: it ignores surrounding space, and Things does not. // Checked in Things 3 with " Personal " against an area called Personal, - // for project add's area and add's list alike: neither matched. - t, err := database.AreaTarget(area) + // for project add's area and add's list alike: neither matched. So the + // trimmed name is tried, and sent, only when the name as given matches + // nothing. + t, sent, err := areaTargetName(database, area) switch { case err != nil: return db.Target{}, false case t.UUID == "": fmt.Fprintf(d.errOut(), "warning: %s\n", noTarget("area", area, false, fallback)) } + *areap = sent return t, true } @@ -132,7 +138,7 @@ func (c *ProjectEditCmd) checkOwn(d *Deps, database *db.DB, task *model.Task) bo // What Things does with an empty area was not checked. return true } - target, read := projectArea(d, *c.Area, "the project will stay where it is") + target, read := projectArea(d, c.Area, "the project will stay where it is") if !read { return true } diff --git a/docs/content/agents.md b/docs/content/agents.md index 273daa7..d76dbe3 100644 --- a/docs/content/agents.md +++ b/docs/content/agents.md @@ -561,7 +561,9 @@ instead of assuming: - **Lists, headings and areas must match.** Things files or moves an item only when `--list`, `--heading` or `--area` matches a project Things shows (open, or closed but not yet logged, which it reopens), an area or - a heading, ignoring case but not surrounding space. A project wins over an + a heading, ignoring case but not surrounding space (`add`, `project add` + and `edit` send a padded name trimmed when only that form matches; + `import` does not trim). A project wins over an area of the same title; when several projects, or several areas, match, Things takes the one whose UUID sorts first, and the CLI notes which on stderr; pass a UUID to choose. A project or area diff --git a/docs/content/commands.md b/docs/content/commands.md index c178290..b84d5a5 100644 --- a/docs/content/commands.md +++ b/docs/content/commands.md @@ -565,7 +565,10 @@ checked. Things files the to-do by matching `--list` (or `--project`) against the titles of the projects and areas it shows, and `--heading` against the headings of that project. Both ignore case but not surrounding space, so -`" Tools "` does not match a project called Tools. When nothing matches it +the CLI sends a name as you typed it when Things matches it that way, and +trimmed when only the trimmed name matches: `" Tools "` goes as `Tools`, +while `"Errands "` still reaches an area whose title really is `Errands `. +A name neither form matches goes as typed, with the warning below. When nothing matches it does not complain: it puts the to-do in the Inbox, or adds it to the list without the heading. A trashed project does not match by title, nor does a closed one Things has moved to the Logbook; one closed but not yet logged @@ -596,7 +599,8 @@ note: "Tools" is in the Trash; Things will file into it there takes an area name or UUID; a UUID goes to Things as `area-id`, since Things matches `area` by title only. It matches the title ignoring case and compatibility forms (fullwidth letters, superscript digits) but not -surrounding space; when several areas match, Things takes the one whose +surrounding space, so the CLI sends a padded name trimmed when only the +trimmed name matches, as for `add --list`; when several areas match, Things takes the one whose UUID sorts first, as for `add --list`, and the CLI notes which. When nothing matches it creates the project with no area. `project add` warns on stderr when that will happen, then sends the project anyway: @@ -661,7 +665,8 @@ things edit 3 --list Tools --heading Setup `--list` and `--heading` move the to-do. Things matches them the way it does for `add`: ignoring case and compatibility forms but not surrounding -space, and only against +space (the CLI sends a padded name trimmed when only that form matches, as +`add` does, and `project edit --area` does the same), and only against the projects (including one closed but not yet logged) and areas `add` matches; when several share the title, the one whose UUID sorts first is the one the to-do moves to. `--list` also takes a UUID of a project or @@ -1067,7 +1072,9 @@ it, padding included: trashed one takes the to-do into the Trash. - A `list` title goes to the list Things picks for that title, as for `add`, under its `heading` when it has one. Things does not trim it, so - `"Work "` does not match `Work`. A title that matches nothing, or a uuid + `"Work "` does not match `Work`. Unlike `add` and `edit`, `import` does + not trim `list`, `area` or `heading`: the payload is Things' own format + and goes as given, so fix the name in the payload when the CLI warns. A title that matches nothing, or a uuid given as `list`, puts the to-do in the Inbox. When a project created earlier in the same payload, or renamed to that title by an earlier update item, has that title, the to-do may go to either, so any list with diff --git a/internal/skill/SKILL.md b/internal/skill/SKILL.md index 8a0a177..1d74369 100644 --- a/internal/skill/SKILL.md +++ b/internal/skill/SKILL.md @@ -105,7 +105,7 @@ already exists: Work Both routes create over AppleScript, so Things3 must be running, and both skip names that already exist, matching case-insensitively as Things does; `tag add` reports a name that differs only in case from an earlier argument as skipped too. `tag add` then re-reads the tag list and exits non-zero if a creation did not land; `--no-verify` (or `no_verify = true`) skips that check, so a dropped creation would be reported as success. -`add` warns the same way when `--list`/`--project` names no project or area by title or UUID (by title, an open project or one closed but not yet logged, which Things still shows and reopens; by UUID, any project) (Things puts the to-do in the Inbox) or `--heading` names no heading of that project (Things drops the heading). Things ignores case in these names, and compatibility forms such as fullwidth letters in project and area titles, but not surrounding space. When several projects (or areas) match, Things takes the one whose UUID sorts first, even if another title matches exactly; a `note:` line on stderr names it, and another says when the project is closed or trashed. Pass a UUID to choose. The add still goes ahead; fix the name and move the to-do if it matters. +`add` warns the same way when `--list`/`--project` names no project or area by title or UUID (by title, an open project or one closed but not yet logged, which Things still shows and reopens; by UUID, any project) (Things puts the to-do in the Inbox) or `--heading` names no heading of that project (Things drops the heading). Things ignores case in these names, and compatibility forms such as fullwidth letters in project and area titles, but not surrounding space, so `add`, `project add` and `edit` send a name trimmed when only the trimmed form matches (a title that really has surrounding space still matches as typed); `import` sends its payload's names as given. When several projects (or areas) match, Things takes the one whose UUID sorts first, even if another title matches exactly; a `note:` line on stderr names it, and another says when the project is closed or trashed. Pass a UUID to choose. The add still goes ahead; fix the name and move the to-do if it matters. `project add --area` takes an area name or UUID. It warns the same way when `--area` names no area (Things creates the project with no area), and still creates the project.