Change BookShelves.Shelf to Return (*Shelf, bool) and Remove StructIsEmpty

CONTRIBUTING.md

@@ -1,12 +1,12 @@
-# Contributing to polybook
+# Contributing to book
 
-Thanks for considering a contribution! polybook is a terminal-native bookmark manager — keeping the CLI fast, the TUI pleasant, and the TOML storage predictable is the top priority.
+Thanks for considering a contribution! book is a terminal-native bookmark manager — keeping the CLI fast, the TUI pleasant, and the TOML storage predictable is the top priority.
 
 ## Quick Start
 
 ```bash
 git clone <your-fork>
-cd polybook
+cd book
 go build ./...
 go test ./...
 go vet ./...
@@ -112,7 +112,7 @@ Note: there is no `.golangci.yml` in the repo yet. If you want to add one, open
 
 There are currently **zero tests**. New features or bug fixes **should** include tests where feasible. Priority targets for coverage:
 
-- `VerifyUniqueURL`, `DedupUnique`, `MergeTags`, `StructIsEmpty`, `GenerateID` in `internal/book`
+- `VerifyUniqueURL`, `DedupUnique`, `MergeTags`, `GenerateID` in `internal/book`
 - TOML round-trip encoding/decoding in `internal/catalog`
 - `WebsiteTitle` / `OpenURL` in `internal/web` (mock HTTP server)
 

VERSION

@@ -1 +1 @@
-v1.3.1
+v1.4.0

cmd/book/actions_test.go

@@ -29,14 +29,23 @@ func loadShelves(t *testing.T, config *book.Config) *book.BookShelves {
 	return &bs
 }
 
+func testShelf(t *testing.T, bs *book.BookShelves, name string) *book.Shelf {
+	t.Helper()
+	shelf, ok := bs.Shelf(name)
+	if !ok {
+		t.Fatalf("shelf %q not found", name)
+	}
+	return shelf
+}
+
 func seedShelf(t *testing.T, config *book.Config, name, collection string) *book.BookShelves {
 	t.Helper()
 	bs := &book.BookShelves{}
 	if err := addShelf(bs, name, "test shelf", config); err != nil {
 		t.Fatalf("seed shelf: %v", err)
 	}
 	if collection != "" {
-		shelf := bs.Shelf(name)
+		shelf := testShelf(t, bs, name)
 		col, err := book.NewCollection(shelf, collection, "")
 		if err != nil {
 			t.Fatalf("seed collection: %v", err)
@@ -60,7 +69,7 @@ func TestAddShelf(t *testing.T) {
 	if len(*bs) != 1 {
 		t.Fatalf("got %d shelves, want 1", len(*bs))
 	}
-	shelf := bs.Shelf("dev")
+	shelf := testShelf(t, bs, "dev")
 	if shelf.Name != "dev" {
 		t.Errorf("Name = %q, want dev", shelf.Name)
 	}
@@ -87,7 +96,7 @@ func TestRemoveShelf(t *testing.T) {
 	if err := removeShelf(bs, "dev", false); err == nil {
 		t.Fatal("expected error without --confirm")
 	}
-	if _, err := os.Stat(bs.Shelf("dev").FilePath); err != nil {
+	if _, err := os.Stat(testShelf(t, bs, "dev").FilePath); err != nil {
 		t.Fatal("shelf file removed before confirm")
 	}
 
@@ -115,13 +124,13 @@ func TestAddCollection(t *testing.T) {
 		t.Fatalf("addCollection error: %v", err)
 	}
 
-	shelf := bs.Shelf("dev")
+	shelf := testShelf(t, bs, "dev")
 	if shelf.Collection("docs") == nil {
 		t.Fatal("collection not found in memory")
 	}
 
 	reloaded := loadShelves(t, config)
-	if reloaded.Shelf("dev").Collection("docs") == nil {
+	if testShelf(t, reloaded, "dev").Collection("docs") == nil {
 		t.Fatal("collection not persisted")
 	}
 
@@ -139,12 +148,12 @@ func TestRemoveCollection(t *testing.T) {
 		t.Fatalf("removeCollection error: %v", err)
 	}
 
-	if bs.Shelf("dev").Collection("docs") != nil {
+	if testShelf(t, bs, "dev").Collection("docs") != nil {
 		t.Error("collection still in memory")
 	}
 
 	reloaded := loadShelves(t, config)
-	if reloaded.Shelf("dev").Collection("docs") != nil {
+	if testShelf(t, reloaded, "dev").Collection("docs") != nil {
 		t.Error("collection not removed from disk")
 	}
 
@@ -163,7 +172,7 @@ func TestAddMark(t *testing.T) {
 	}
 
 	reloaded := loadShelves(t, config)
-	collection := reloaded.Shelf("dev").Collection("docs")
+	collection := testShelf(t, reloaded, "dev").Collection("docs")
 	if len(collection.Marks) != 1 {
 		t.Fatalf("got %d marks, want 1", len(collection.Marks))
 	}
@@ -188,7 +197,7 @@ func TestGetMark(t *testing.T) {
 		t.Fatalf("addMark error: %v", err)
 	}
 	bs = loadShelves(t, config)
-	mark := bs.Shelf("dev").Collection("docs").Marks[0]
+	mark := testShelf(t, bs, "dev").Collection("docs").Marks[0]
 
 	// By ID.
 	if err := getMark(bs, mark.ID, "", "", config); err != nil {
@@ -218,14 +227,14 @@ func TestEditMark(t *testing.T) {
 		t.Fatalf("addMark error: %v", err)
 	}
 	bs = loadShelves(t, config)
-	mark := bs.Shelf("dev").Collection("docs").Marks[0]
+	mark := testShelf(t, bs, "dev").Collection("docs").Marks[0]
 
 	if err := editMark(bs, mark.ID, "Updated", "go,cli", "", config); err != nil {
 		t.Fatalf("editMark error: %v", err)
 	}
 
 	reloaded := loadShelves(t, config)
-	updated := reloaded.Shelf("dev").Collection("docs").Marks[0]
+	updated := testShelf(t, reloaded, "dev").Collection("docs").Marks[0]
 	if updated.Name != "Updated" {
 		t.Errorf("Name = %q, want Updated", updated.Name)
 	}
@@ -244,7 +253,7 @@ func TestEditMarkURLCollision(t *testing.T) {
 		t.Fatalf("addMark two error: %v", err)
 	}
 	bs = loadShelves(t, config)
-	marks := bs.Shelf("dev").Collection("docs").Marks
+	marks := testShelf(t, bs, "dev").Collection("docs").Marks
 	oneID := marks[0].ID
 
 	// Changing mark one to mark two's URL should fail before mutating.
@@ -269,7 +278,7 @@ func TestRemoveMark(t *testing.T) {
 		t.Fatalf("addMark error: %v", err)
 	}
 	bs = loadShelves(t, config)
-	mark := bs.Shelf("dev").Collection("docs").Marks[0]
+	mark := testShelf(t, bs, "dev").Collection("docs").Marks[0]
 
 	if err := removeMark(bs, mark.ID, true, config); err != nil {
 		t.Fatalf("removeMark error: %v", err)

cmd/book/collection.go

@@ -39,8 +39,8 @@ func addCollection(bs *book.BookShelves, shelfName, collectionName, description
 		return runProgram(collectionRootScreen(bs, "add", config))
 	}
 
-	shelf := bs.Shelf(shelfName)
-	if shelf == nil || book.StructIsEmpty(shelf) {
+	shelf, ok := bs.Shelf(shelfName)
+	if !ok {
 		return fmt.Errorf("shelf %q not found", shelfName)
 	}
 
@@ -65,8 +65,8 @@ func removeCollection(bs *book.BookShelves, shelfName, collectionName string, co
 		return fmt.Errorf("remove collection requires --confirm")
 	}
 
-	shelf := bs.Shelf(shelfName)
-	if shelf == nil || book.StructIsEmpty(shelf) {
+	shelf, ok := bs.Shelf(shelfName)
+	if !ok {
 		return fmt.Errorf("shelf %q not found", shelfName)
 	}
 

cmd/book/mark.go

@@ -15,7 +15,7 @@ func getMark(bs *book.BookShelves, id, url, format string, config *book.Config)
 		if !config.Interactive {
 			return fmt.Errorf("missing required flag: --id or --url")
 		}
-		return runProgram(markRootScreen(bs, &book.Mark{}, "get", config))
+		return runProgram(markRootScreen(bs, nil, "get", config))
 	}
 
 	var target *book.Mark
@@ -83,15 +83,15 @@ func marks(bs *book.BookShelves, shelfName string, collectionName string, format
 		}
 		return book.PrintCatalog(collection, format)
 	}
-	return runProgram(markRootScreen(bs, &book.Mark{}, "list", config))
+	return runProgram(markRootScreen(bs, nil, "list", config))
 }
 
 func editMark(bs *book.BookShelves, id, title, tags, url string, config *book.Config) error {
 	if err := requireFlag("id", id); err != nil {
 		if !config.Interactive {
 			return err
 		}
-		return runProgram(markRootScreen(bs, &book.Mark{}, "edit", config))
+		return runProgram(markRootScreen(bs, nil, "edit", config))
 	}
 
 	target := bs.FindMarkByID(id)
@@ -179,12 +179,12 @@ func addMark(bs *book.BookShelves, URL string, tags string, shelfName string, co
 
 	// Non-interactive path: all required flags provided
 	if shelfName != "" && collectionName != "" {
-		shelf := bs.Shelf(shelfName)
-		if shelf == nil || book.StructIsEmpty(shelf) {
+		shelf, ok := bs.Shelf(shelfName)
+		if !ok {
 			return fmt.Errorf("shelf %q not found", shelfName)
 		}
 		collection := shelf.Collection(collectionName)
-		if collection == nil || book.StructIsEmpty(collection) {
+		if collection == nil {
 			return fmt.Errorf("collection %q not found in shelf %q", collectionName, shelfName)
 		}
 		mark.Shelf = shelf
@@ -205,7 +205,7 @@ func removeMark(bs *book.BookShelves, id string, confirmed bool, config *book.Co
 		if !config.Interactive {
 			return err
 		}
-		return runProgram(markRootScreen(bs, &book.Mark{}, "delete", config))
+		return runProgram(markRootScreen(bs, nil, "delete", config))
 	}
 
 	if !confirmed {
@@ -236,12 +236,12 @@ func restoreMark(bs *book.BookShelves, id string, shelfName string, collectionNa
 		return fmt.Errorf("restore requires --id, or --shelf/--collection/--url")
 	}
 
-	shelf := bs.Shelf(shelfName)
-	if book.StructIsEmpty(shelf) {
+	shelf, ok := bs.Shelf(shelfName)
+	if !ok {
 		return fmt.Errorf("shelf %q not found", shelfName)
 	}
 	collection := shelf.Collection(collectionName)
-	if book.StructIsEmpty(collection) {
+	if collection == nil {
 		return fmt.Errorf("collection %q not found in shelf %q", collectionName, shelfName)
 	}
 
@@ -266,10 +266,6 @@ func clearSoftDelete(target *book.Mark) error {
 }
 
 func markRootScreen(bs *book.BookShelves, mark *book.Mark, action string, config *book.Config) model.RootScreen {
-	if book.StructIsEmpty(mark) {
-		screen := model.GetMarkForm(bs, &book.Mark{}, config, action)
-		return model.RootScreen{Model: &screen}
-	}
 	screen := model.GetMarkForm(bs, mark, config, action)
 	return model.RootScreen{Model: &screen}
 }

internal/book/types.go

@@ -9,7 +9,6 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"reflect"
 	"slices"
 	"strings"
 	"time"
@@ -132,14 +131,15 @@ func (bs *BookShelves) AddShelf(shelf Shelf) {
 	*bs = append(*bs, shelf)
 }
 
-// Shelf returns a shelf by name, or a zero-value Shelf if not found.
-func (bs *BookShelves) Shelf(s string) *Shelf {
+// Shelf returns a shelf by name. The second result is true if the shelf was
+// found and false otherwise.
+func (bs *BookShelves) Shelf(s string) (*Shelf, bool) {
 	for i := range *bs {
 		if (*bs)[i].Name == s {
-			return &(*bs)[i]
+			return &(*bs)[i], true
 		}
 	}
-	return &Shelf{}
+	return nil, false
 }
 
 // ShelfNames returns the names of all loaded shelves.
@@ -465,19 +465,6 @@ func DedupUnique[T comparable](slice ...[]T) []T {
 	return unique
 }
 
-// StructIsEmpty reports whether the given struct pointer is nil or contains only zero values.
-func StructIsEmpty[T any](ptr *T) bool {
-	if ptr == nil {
-		return true
-	}
-
-	val := reflect.ValueOf(ptr).Elem()
-
-	// This will return true if all fields within the struct have their
-	// zero values (e.g., 0 for int, "" for string, nil for pointers, etc.).
-	return val.IsZero()
-}
-
 // GenerateID returns the first 8 hex characters of the SHA-256 hash of a URL.
 func GenerateID(url string) string {
 	hash := sha256.Sum256([]byte(url))

internal/book/types_test.go

@@ -46,35 +46,17 @@ func TestDedupUnique(t *testing.T) {
 	}
 }
 
-func TestStructIsEmpty(t *testing.T) {
-	tests := []struct {
-		name string
-		ptr  *Mark
-		want bool
-	}{
-		{
-			name: "nil pointer",
-			ptr:  nil,
-			want: true,
-		},
-		{
-			name: "zero struct",
-			ptr:  &Mark{},
-			want: true,
-		},
-		{
-			name: "non-zero struct",
-			ptr:  &Mark{Name: "example"},
-			want: false,
-		},
+func TestBookShelvesShelf(t *testing.T) {
+	bs := BookShelves{
+		{Name: "shelf-a"},
 	}
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := StructIsEmpty(tt.ptr); got != tt.want {
-				t.Errorf("StructIsEmpty() = %v, want %v", got, tt.want)
-			}
-		})
+	if shelf, ok := bs.Shelf("shelf-a"); !ok || shelf == nil || shelf.Name != "shelf-a" {
+		t.Errorf("expected shelf-a, got %v, ok=%v", shelf, ok)
+	}
+
+	if shelf, ok := bs.Shelf("missing"); ok || shelf != nil {
+		t.Errorf("expected missing shelf to return nil/false, got %v, ok=%v", shelf, ok)
 	}
 }
 

internal/model/collection_model.go

@@ -61,7 +61,8 @@ func (m getCollectionModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	}
 
 	if m.get.book.form.State == huh.StateCompleted {
-		m.get.shelf = m.get.book.shelves.Shelf(m.get.book.form.GetString("shelf"))
+		shelf, _ := m.get.book.shelves.Shelf(m.get.book.form.GetString("shelf"))
+		m.get.shelf = shelf
 		switch m.action {
 		case "add":
 			editScreen := editCollectionForm(m.get.book.shelves, m.get.shelf, m.get.config, m.action)
@@ -153,9 +154,13 @@ func GetCollectionForm(bs *book.BookShelves, config *book.Config, action string)
 		huh.NewGroup(
 			huh.NewSelect[string]().
 				Title("Pick your collection.").
-				Options(
-					huh.NewOptions(bs.Shelf(chosenShelf).CollectionsNames()...)...,
-				).
+				OptionsFunc(func() []huh.Option[string] {
+					shelf, _ := bs.Shelf(chosenShelf)
+					if shelf == nil {
+						return []huh.Option[string]{}
+					}
+					return huh.NewOptions(shelf.CollectionsNames()...)
+				}, &chosenShelf).
 				Key("collection").
 				Value(&chosenCollection),
 		).WithHideFunc(func() bool {