fix(lookup): report a project not found for every ref shape in a project lookup

cmd/things/errors.go

@@ -316,25 +316,35 @@ 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
 }
 
 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

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,20 +113,20 @@ 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) {
 			// The row exists in the cache, so this reference is a row number
 			// 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, &notFoundError{
-				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, &notFound):
-		return nil, notARowError(ref, last, cacheErr)
+		return nil, notARowError(noun, ref, last, cacheErr)
 	case kind == refRowLike && errors.As(err, &notFound):
-		return nil, markedRowError(ref, digits, last, cacheErr)
+		return nil, markedRowError(noun, ref, digits, last, cacheErr)
 	case uuidShaped && errors.As(err, &notFound):
 		return nil, &notFoundError{
-			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,19 +201,19 @@ 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)
 		}
 		fmt.Fprint(&b, "Re-run with a UUID or more specific string.")
 		// 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 &notFoundError{Kind: "task", Query: ref, msg: msg}
+	msg += fmt.Sprintf("pass the %s's uuid or full title", noun)
+	return &notFoundError{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 &notFoundError{Kind: "task", Query: ref, msg: msg}
+	return &notFoundError{Kind: noun, Query: ref, msg: msg}
 }
 
 // cacheTaskUUIDs records the listing's task UUIDs so a later numeric ref

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) {

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