diff --git a/cmd/things/errors.go b/cmd/things/errors.go index 69d2ed5..1861245 100644 --- a/cmd/things/errors.go +++ b/cmd/things/errors.go @@ -316,6 +316,7 @@ func refuseBlankTitle(title, kind, done string) error { // still reaches the candidates for the JSON payload. type ambiguousRefError struct { msg string + kind string // "task", or "project" for a project lookup inner *db.AmbiguousTaskError } @@ -323,18 +324,27 @@ func (e *ambiguousRefError) Error() string { return e.msg } func (e *ambiguousRefError) Unwrap() error { return e.inner } +// refNoun is the item kind a ref error names: kind, or "task" when unset. +func refNoun(kind string) string { + if kind == "" { + return "task" + } + return kind +} + // cacheRef is what the errors refusing a row number over the last-list cache // share: staleCacheError, otherDBCacheError and unreadableCacheError embed // it. They share the "stale list cache" token too, because the remedy is the // same for all three: re-list and use the new row number or the uuid. // Re-listing also rewrites a cache file that could not be read. type cacheRef struct { + Kind string // "task", or "project" for a project lookup Query string // the reference as typed, e.g. "2" } func (e *cacheRef) fillPayload(p *jsonErrorPayload) { p.Error = "stale list cache" - p.Kind = "task" + p.Kind = refNoun(e.Kind) p.Query = e.Query } @@ -350,7 +360,7 @@ type staleCacheError struct { func (e *staleCacheError) Error() string { var b strings.Builder - fmt.Fprintf(&b, "task #%d comes from a stale list cache", e.Row) + fmt.Fprintf(&b, "%s #%d comes from a stale list cache", refNoun(e.Kind), e.Row) if e.Last.WrittenAt.IsZero() { // A file written before 0.8.0 records no time, so there is nothing // to age it by and no listing to name. @@ -363,7 +373,7 @@ func (e *staleCacheError) Error() string { b.WriteString(". Re-run your listing") } } - b.WriteString(" and use the new row number, or pass the task's uuid.") + fmt.Fprintf(&b, " and use the new row number, or pass the %s's uuid.", refNoun(e.Kind)) return b.String() } @@ -377,7 +387,7 @@ type unreadableCacheError struct { } func (e *unreadableCacheError) Error() string { - return fmt.Sprintf("%q may be a row of the last list, but %v; run a listing again or use the task's title or uuid", e.Query, e.Err) + return fmt.Sprintf("%q may be a row of the last list, but %v; run a listing again or use the %s's title or uuid", e.Query, e.Err, refNoun(e.Kind)) } func (e *unreadableCacheError) Unwrap() error { return e.Err } @@ -386,16 +396,17 @@ func (e *unreadableCacheError) Unwrap() error { return e.Err } // it would match every task, or every untitled one, so it is refused before // any lookup. type emptyRefError struct { + Kind string // "task", or "project" for a project lookup Query string } func (e *emptyRefError) Error() string { - return "the task reference is empty; pass a row number, a uuid or a title" + return fmt.Sprintf("the %s reference is empty; pass a row number, a uuid or a title", refNoun(e.Kind)) } func (e *emptyRefError) fillPayload(p *jsonErrorPayload) { p.Error = "empty reference" - p.Kind = "task" + p.Kind = refNoun(e.Kind) p.Query = e.Query } @@ -411,7 +422,7 @@ type otherDBCacheError struct { func (e *otherDBCacheError) Error() string { var b strings.Builder - fmt.Fprintf(&b, "task #%d comes from a listing of a different database", e.Row) + fmt.Fprintf(&b, "%s #%d comes from a listing of a different database", refNoun(e.Kind), e.Row) listing := "the listing" if e.Last.Command != "" { listing = fmt.Sprintf("`%s`", e.Last.Command) @@ -425,7 +436,7 @@ func (e *otherDBCacheError) Error() string { if e.Current != "" { fmt.Fprintf(&b, ", and this command reads %s", e.Current) } - b.WriteString(". Re-run the listing against this database and use the new row number, or pass the task's uuid.") + fmt.Fprintf(&b, ". Re-run the listing against this database and use the new row number, or pass the %s's uuid.", refNoun(e.Kind)) return b.String() } @@ -492,6 +503,9 @@ func errorPayload(err error) jsonErrorPayload { if errors.As(err, &ambig) { payload.Error = "ambiguous task" payload.Kind = "task" + if ref := (*ambiguousRefError)(nil); errors.As(err, &ref) && ref.kind != "" { + payload.Kind = ref.kind + } payload.Query = ambig.Query payload.Matches = matchList(ambig.Matches) return payload diff --git a/cmd/things/lookup.go b/cmd/things/lookup.go index 29afb01..8c26c13 100644 --- a/cmd/things/lookup.go +++ b/cmd/things/lookup.go @@ -85,10 +85,19 @@ func resolveProjectRef(d *Deps, ref string, database *db.DB, write bool) (*model } func resolveRef(d *Deps, ref string, database *db.DB, write, projects bool) (*model.Task, error) { + // noun names what was looked up in an error, so `open --project 99` + // reports a project not found, as a title miss already does. + noun := "task" + lookup, exact := database.GetTask, database.GetTaskExact + if projects { + noun = "project" + lookup, exact = database.GetProject, database.GetProjectExact + } + // A blank ref is a title fragment of every task, and an exact title of // every untitled one: `show ''` must not pick one of those. if strings.TrimSpace(ref) == "" { - return nil, &emptyRefError{Query: ref} + return nil, &emptyRefError{Kind: noun, Query: ref} } // Try numeric index from last list. Surrounding space does not stop a @@ -104,7 +113,7 @@ func resolveRef(d *Deps, ref string, database *db.DB, write, projects bool) (*mo // listing: the user may well have meant a row of it, so a bare number // is refused rather than tried as a title. if kind == refRow && errors.Is(cacheErr, cache.ErrUnreadable) { - return nil, &unreadableCacheError{cacheRef: cacheRef{Query: ref}, Err: cacheErr} + return nil, &unreadableCacheError{cacheRef: cacheRef{Kind: noun, Query: ref}, Err: cacheErr} } if n, err := strconv.Atoi(digits); kind == refRow && err == nil && n >= 1 { if cacheErr == nil && n <= len(last.UUIDs) { @@ -112,12 +121,12 @@ func resolveRef(d *Deps, ref string, database *db.DB, write, projects bool) (*mo // and nothing else. Refuse it when the listing behind it is old // enough that the rows have probably moved (issue #265). if last.Stale(time.Now()) { - return nil, &staleCacheError{cacheRef: cacheRef{Query: ref}, Row: n, Last: last} + return nil, &staleCacheError{cacheRef: cacheRef{Kind: noun, Query: ref}, Row: n, Last: last} } // Nor is a listing of another database a guide to this one // (issue #274). if !cacheFromThisDB(d, last) { - return nil, &otherDBCacheError{cacheRef: cacheRef{Query: ref}, Row: n, Last: last, Current: d.dbIdentity()} + return nil, &otherDBCacheError{cacheRef: cacheRef{Kind: noun, Query: ref}, Row: n, Last: last, Current: d.dbIdentity()} } t, err := database.GetTaskByUUID(last.UUIDs[n-1]) if err != nil { @@ -127,9 +136,9 @@ func resolveRef(d *Deps, ref string, database *db.DB, write, projects bool) (*mo return t, nil } return nil, ¬FoundError{ - Kind: "task", + Kind: noun, Query: ref, - msg: fmt.Sprintf("task #%d no longer exists (stale list cache — re-run list)", n), + msg: fmt.Sprintf("%s #%d no longer exists (stale list cache — re-run list)", noun, n), } } } @@ -146,10 +155,6 @@ func resolveRef(d *Deps, ref string, database *db.DB, write, projects bool) (*mo // in the wrong case would otherwise reach a task whose title merely // contains it. uuidShaped := kind == refPlain && looksLikeUUID(strings.TrimSpace(ref)) - lookup, exact := database.GetTask, database.GetTaskExact - if projects { - lookup, exact = database.GetProject, database.GetProjectExact - } if kind != refPlain || uuidShaped { lookup = exact } @@ -178,14 +183,14 @@ func resolveRef(d *Deps, ref string, database *db.DB, write, projects bool) (*mo switch { case kind == refRow && errors.As(err, ¬Found): - return nil, notARowError(ref, last, cacheErr) + return nil, notARowError(noun, ref, last, cacheErr) case kind == refRowLike && errors.As(err, ¬Found): - return nil, markedRowError(ref, digits, last, cacheErr) + return nil, markedRowError(noun, ref, digits, last, cacheErr) case uuidShaped && errors.As(err, ¬Found): return nil, ¬FoundError{ - Kind: "task", + Kind: noun, Query: ref, - msg: fmt.Sprintf("%q looks like a uuid, but no item has that uuid and no task has exactly that title; uuids are case-sensitive, so copy the uuid exactly as a listing prints it. If you meant part of a title, pass a shorter or longer part of it, or the full title", ref), + msg: fmt.Sprintf("%q looks like a uuid, but no item has that uuid and no %s has exactly that title; uuids are case-sensitive, so copy the uuid exactly as a listing prints it. If you meant part of a title, pass a shorter or longer part of it, or the full title", ref, noun), } } @@ -196,7 +201,7 @@ func resolveRef(d *Deps, ref string, database *db.DB, write, projects bool) (*mo if !d.interactive() { var b strings.Builder - fmt.Fprintf(&b, "ambiguous task %q — matches %d tasks:\n", ambig.Query, len(ambig.Matches)) + fmt.Fprintf(&b, "ambiguous %s %q — matches %s:\n", noun, ambig.Query, plural(len(ambig.Matches), noun)) for i, m := range ambig.Matches { fmt.Fprintf(&b, " %d. %s [%s] (%s)\n", i+1, output.OneLine(m.Title), m.Type, m.UUID) } @@ -204,11 +209,11 @@ func resolveRef(d *Deps, ref string, database *db.DB, write, projects bool) (*mo // Wrap rather than replace: the plain-text reader gets the rendered // list, while --json still reaches the candidates behind it // (issue #152). - return nil, &ambiguousRefError{msg: b.String(), inner: ambig} + return nil, &ambiguousRefError{msg: b.String(), kind: noun, inner: ambig} } // Interactive: prompt user to pick - fmt.Fprintf(d.errOut(), "Multiple tasks match %q:\n", ambig.Query) + fmt.Fprintf(d.errOut(), "Multiple %ss match %q:\n", noun, ambig.Query) for i, m := range ambig.Matches { project := "" if m.ProjectTitle != "" { @@ -366,32 +371,34 @@ func rowMarker(prefix string) bool { } // markedRowError refuses a row-like ref that is not the exact title of a -// task. It suggests the bare number only when the last list has a row N. -func markedRowError(ref, digits string, last cache.LastList, cacheErr error) error { - msg := fmt.Sprintf("%q is not a row reference and no task has exactly that title; ", ref) +// task, or of a project when noun says so. It suggests the bare number only +// when the last list has a row N. +func markedRowError(noun, ref, digits string, last cache.LastList, cacheErr error) error { + msg := fmt.Sprintf("%q is not a row reference and no %s has exactly that title; ", ref, noun) if n, err := strconv.Atoi(digits); err == nil && cacheErr == nil && n >= 1 && n <= len(last.UUIDs) { msg += fmt.Sprintf("if you meant row %d of the last list, use %d, otherwise ", n, n) } - msg += "pass the task's uuid or full title" - return ¬FoundError{Kind: "task", Query: ref, msg: msg} + msg += fmt.Sprintf("pass the %s's uuid or full title", noun) + return ¬FoundError{Kind: noun, Query: ref, msg: msg} } // notARowError refuses an all-digit ref that is not a row in the last list and -// is not the exact title of a task (issue #375). It reads as a not-found, and -// says to re-run the list rather than leave the user to guess what was tried. -func notARowError(ref string, last cache.LastList, cacheErr error) error { +// is not the exact title of a task, or of a project when noun says so (issue +// #375). It reads as a not-found, and says to re-run the list rather than +// leave the user to guess what was tried. +func notARowError(noun, ref string, last cache.LastList, cacheErr error) error { msg := fmt.Sprintf("%q is not a row in the last list", ref) if cacheErr != nil { - msg = fmt.Sprintf("no task is titled %q, and there is no last list for it to be a row of", ref) + msg = fmt.Sprintf("no %s is titled %q, and there is no last list for it to be a row of", noun, ref) } else if len(last.UUIDs) > 0 { msg += fmt.Sprintf(" (it has %s)", plural(len(last.UUIDs), "row")) } if cacheErr == nil && last.Command != "" { - msg += fmt.Sprintf(". Re-run `%s` and use the new row number, or pass the task's uuid or full title.", last.Command) + msg += fmt.Sprintf(". Re-run `%s` and use the new row number, or pass the %s's uuid or full title.", last.Command, noun) } else { - msg += ". Re-run your listing and use the row number, or pass the task's uuid or full title." + msg += fmt.Sprintf(". Re-run your listing and use the row number, or pass the %s's uuid or full title.", noun) } - return ¬FoundError{Kind: "task", Query: ref, msg: msg} + return ¬FoundError{Kind: noun, Query: ref, msg: msg} } // cacheTaskUUIDs records the listing's task UUIDs so a later numeric ref diff --git a/cmd/things/run_test.go b/cmd/things/run_test.go index bb7bc5d..9b1c557 100644 --- a/cmd/things/run_test.go +++ b/cmd/things/run_test.go @@ -978,6 +978,83 @@ func TestRunProjectEditNoProjectAmongTodos(t *testing.T) { } } +// A row number, a row-like ref or a uuid-shaped ref that names no project is +// a project not found, as a title miss is: `open --project 99` must not say +// task not found. +func TestRunProjectRefNotFoundKind(t *testing.T) { + refs := []struct{ name, ref, want string }{ + {"row past the list", "99", `"99" is not a row in the last list`}, + {"stale row", "2", "project #2 no longer exists"}, + {"row-like", "#12", "no project has exactly that title"}, + {"uuid-shaped", "Abcdefghijklmnopqrstu", "no project has exactly that title"}, + } + cmds := map[string][]string{ + "open": {"open", "--project"}, + "project edit": {"project", "edit"}, + } + for _, r := range refs { + for name, cmd := range cmds { + t.Run(r.name+" "+name, func(t *testing.T) { + isolateHome(t) + seedCache(t, time.Minute, "things projects", "proj-v2", "gone") + sqlDB := dbtest.NewSQL(t) + dbtest.NewFixture(t, sqlDB).Project("proj-v2", "Launch v2", 0) + captured := stubExec(t) + args := append(append([]string{}, cmd...), r.ref) + if name == "project edit" { + args = append(args, "--notes", "x") + } + err := runWith(t, db.NewFromSQL(sqlDB), args...) + if len(*captured) != 0 { + t.Errorf("nothing should be sent, got %v", *captured) + } + if err == nil || !strings.Contains(err.Error(), r.want) { + t.Errorf("err = %v, want it to contain %q", err, r.want) + } + if err != nil && strings.Contains(err.Error(), "task") { + t.Errorf("err = %q, says task for a project lookup", err) + } + payload, raw := decodePayload(t, err) + if payload.Error != "not found" || payload.Kind != "project" || payload.Query != r.ref { + t.Errorf("payload = %s, want a project not found", raw) + } + }) + } + } +} + +// The other refusals of a project ref name a project too: a stale row, an +// empty ref and a title several projects share. +func TestRunProjectRefRefusalKind(t *testing.T) { + cases := []struct{ name, ref, token, want string }{ + {"stale row", "1", "stale list cache", "project #1 comes from a stale list cache"}, + {"empty", " ", "empty reference", "the project reference is empty"}, + {"ambiguous", "Launch", "ambiguous task", `ambiguous project "Launch" — matches 2 projects`}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + isolateHome(t) + seedCache(t, 5*time.Hour, "things projects", "proj-v2") + sqlDB := dbtest.NewSQL(t) + f := dbtest.NewFixture(t, sqlDB) + f.Project("proj-v2", "Launch v2", 0) + f.Project("proj-v3", "Launch v3", 1) + stubExec(t) + err := runWith(t, db.NewFromSQL(sqlDB), "open", "--project", c.ref) + if err == nil || !strings.Contains(err.Error(), c.want) { + t.Fatalf("err = %v, want it to contain %q", err, c.want) + } + if strings.Contains(err.Error(), "task") { + t.Errorf("err = %q, says task for a project lookup", err) + } + payload, raw := decodePayload(t, err) + if payload.Error != c.token || payload.Kind != "project" { + t.Errorf("payload = %s, want %q of kind project", raw, c.token) + } + }) + } +} + // The write rules still hold for a title only a closed project carries: the // edit is refused, not sent to it. func TestRunProjectEditRefusesClosedProjectTitle(t *testing.T) { diff --git a/docs/content/agents.md b/docs/content/agents.md index b2af4c9..5f686ae 100644 --- a/docs/content/agents.md +++ b/docs/content/agents.md @@ -323,7 +323,7 @@ token for a write whose exact title only closed items carry, which is refused whatever the write was. `not a task` is a project handed to `edit`, and `not a project` a task handed to `project edit` or `open --project`; both refuse before anything is written -or opened and name the command to retry with. `trashed` is a row number, uuid or exact title that named an +or opened and name the command to retry with. Any other refusal of a project reference (`not found`, `ambiguous task`, `stale list cache`, `empty reference`) carries `kind` `project`. `trashed` is a row number, uuid or exact title that named an item in the Trash, or a task whose project is in the Trash (Things shows it only in the Trash); the write is refused before anything is sent. The two import failures carry an `items` array naming which payload items were blocked or did not land; the [Commands](/commands/) page diff --git a/internal/skill/SKILL.md b/internal/skill/SKILL.md index e485e15..262dddf 100644 --- a/internal/skill/SKILL.md +++ b/internal/skill/SKILL.md @@ -68,7 +68,7 @@ Every command that reads or writes Things accepts `--json` / `-j`; `version`, `c {"error": "error", "message": "..."} ``` -On `ambiguous task`, retry with one of `matches[].uuid`; each candidate carries its `type`, and a title a project and a to-do share is reported this way by `edit`, `show`, `complete` and `cancel` rather than resolving to one of them, so `type` tells you whether the uuid you picked wants `things edit` or `things project edit`. `project edit` and `open --project` match titles against projects only, so a to-do sharing the title does not get in the way; when no project matches, a title or fragment only one to-do matches (in the Trash or not) is refused with `not a project`, and anything else is `not found` with `kind` `project`. On `stale list cache` a row number was used past its four hours or against a different database; re-list and use the `uuid`. On `not a task` the reference resolved to a project, `edit` wrote nothing, and the retry is `things project edit `; `not a project` is the same mistake the other way round, and the retry is `things edit ` (from `open --project`, it is `things open `, and nothing was opened). On `empty reference` the `` argument was empty or only spaces; nothing was looked up. On `trashed` the row number, uuid or exact title named an item in the Trash (often one trashed in Things after the listing was printed), or named a task whose project is in the Trash (Things shows such a task only in the Trash, so the CLI refuses it too; the message names the project, and a title fragment never matches it); `complete`, `cancel`, `edit` and `project edit` refuse it and send nothing. Re-list and check you have the item you meant: do not retry with the same reference. `show` still reaches a trashed item and says so (`"trashed": true`, or `Status: Open (in Trash)` in plain output). On `already closed` or `trashed` with a `query`, the reference was a title that no open item has, once case and surrounding space are set aside, and the write was refused; the object names the most recent closed or trashed item with that title and, when there are several, lists them all in `matches`. Re-list and use the `uuid` of the item you meant. The same tokens from a uuid or row number carry no `query`. A `trashed` error about a task whose project is in the Trash names it in `project`. On `blank-title` the title of an `add`, `project add`, or an `edit`/`project edit` `--title`, was empty or only whitespace: nothing was sent (Things would leave the item untitled), so give it a title. Otherwise on `already closed` you asked to cancel a completed item or complete a cancelled one; nothing was sent, and switching it is for the user to do in Things (closing it the way it already is exits 0 with a note instead). This covers argument and flag errors too — `things --json show` with no argument returns the object, not a usage block. Without `--json`, errors stay a plain `Error: ...` line on stderr. +On `ambiguous task`, retry with one of `matches[].uuid`; each candidate carries its `type`, and a title a project and a to-do share is reported this way by `edit`, `show`, `complete` and `cancel` rather than resolving to one of them, so `type` tells you whether the uuid you picked wants `things edit` or `things project edit`. `project edit` and `open --project` match titles against projects only, so a to-do sharing the title does not get in the way; when no project matches, a title or fragment only one to-do matches (in the Trash or not) is refused with `not a project`, and anything else is `not found` with `kind` `project`. Their other refusals (`ambiguous task`, `stale list cache`, `empty reference`) carry `kind` `project` too. On `stale list cache` a row number was used past its four hours or against a different database; re-list and use the `uuid`. On `not a task` the reference resolved to a project, `edit` wrote nothing, and the retry is `things project edit `; `not a project` is the same mistake the other way round, and the retry is `things edit ` (from `open --project`, it is `things open `, and nothing was opened). On `empty reference` the `` argument was empty or only spaces; nothing was looked up. On `trashed` the row number, uuid or exact title named an item in the Trash (often one trashed in Things after the listing was printed), or named a task whose project is in the Trash (Things shows such a task only in the Trash, so the CLI refuses it too; the message names the project, and a title fragment never matches it); `complete`, `cancel`, `edit` and `project edit` refuse it and send nothing. Re-list and check you have the item you meant: do not retry with the same reference. `show` still reaches a trashed item and says so (`"trashed": true`, or `Status: Open (in Trash)` in plain output). On `already closed` or `trashed` with a `query`, the reference was a title that no open item has, once case and surrounding space are set aside, and the write was refused; the object names the most recent closed or trashed item with that title and, when there are several, lists them all in `matches`. Re-list and use the `uuid` of the item you meant. The same tokens from a uuid or row number carry no `query`. A `trashed` error about a task whose project is in the Trash names it in `project`. On `blank-title` the title of an `add`, `project add`, or an `edit`/`project edit` `--title`, was empty or only whitespace: nothing was sent (Things would leave the item untitled), so give it a title. Otherwise on `already closed` you asked to cancel a completed item or complete a cancelled one; nothing was sent, and switching it is for the user to do in Things (closing it the way it already is exits 0 with a note instead). This covers argument and flag errors too — `things --json show` with no argument returns the object, not a usage block. Without `--json`, errors stay a plain `Error: ...` line on stderr. `import` fails per item, so its two failures add an `items` array — act on that rather than parsing `message`: