Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions cmd/things/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, &params.List, &params.Heading)
if target.ByUUID {
params.List, params.ListID = "", target.UUID
}
Expand Down Expand Up @@ -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"))
Expand All @@ -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{}
Expand All @@ -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)
}
Expand Down
7 changes: 4 additions & 3 deletions cmd/things/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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", "Personal", "", inArea1},
Expand Down
27 changes: 24 additions & 3 deletions cmd/things/edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -222,17 +230,30 @@ 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
case !f:
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
Expand Down
6 changes: 3 additions & 3 deletions cmd/things/edit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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},
Expand Down Expand Up @@ -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},
Expand Down
71 changes: 71 additions & 0 deletions cmd/things/names.go
Original file line number Diff line number Diff line change
@@ -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 != ""
}
67 changes: 67 additions & 0 deletions cmd/things/names_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
18 changes: 12 additions & 6 deletions cmd/things/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, &params.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}
Expand All @@ -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
}
Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 3 additions & 1 deletion docs/content/agents.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading