From 089361c120ed82a6f2bfe4c56ac72787aab9dac4 Mon Sep 17 00:00:00 2001 From: Aaron Martell Date: Wed, 9 Sep 2026 12:31:28 -0500 Subject: [PATCH 1/2] feat: shape shelf API --- CONTRIBUTING.md | 8 ++-- cmd/book/actions_test.go | 35 ++++++++++------- cmd/book/collection.go | 8 ++-- cmd/book/mark.go | 24 +++++------- internal/book/types.go | 23 +++-------- internal/book/types_test.go | 36 +++++------------- internal/model/collection_model.go | 13 +++++-- internal/model/mark_model.go | 61 +++++++++++++++++++++++------- 8 files changed, 111 insertions(+), 97 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3d10fbf..5043c40 100644 --- a/CONTRIBUTING.md +++ b/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 -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) diff --git a/cmd/book/actions_test.go b/cmd/book/actions_test.go index 8b2dc4d..f3917d1 100644 --- a/cmd/book/actions_test.go +++ b/cmd/book/actions_test.go @@ -29,6 +29,15 @@ 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{} @@ -36,7 +45,7 @@ func seedShelf(t *testing.T, config *book.Config, name, collection string) *book 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) diff --git a/cmd/book/collection.go b/cmd/book/collection.go index 8ca089b..500e5c7 100644 --- a/cmd/book/collection.go +++ b/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) } diff --git a/cmd/book/mark.go b/cmd/book/mark.go index 6137b8f..5e6913b 100644 --- a/cmd/book/mark.go +++ b/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,7 +83,7 @@ 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 { @@ -91,7 +91,7 @@ func editMark(bs *book.BookShelves, id, title, tags, url string, config *book.Co 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} } diff --git a/internal/book/types.go b/internal/book/types.go index f43e9f6..1de4635 100644 --- a/internal/book/types.go +++ b/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)) diff --git a/internal/book/types_test.go b/internal/book/types_test.go index 74de960..7343e2e 100644 --- a/internal/book/types_test.go +++ b/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) } } diff --git a/internal/model/collection_model.go b/internal/model/collection_model.go index 3942eeb..fd33028 100644 --- a/internal/model/collection_model.go +++ b/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 { diff --git a/internal/model/mark_model.go b/internal/model/mark_model.go index 3493a38..09f4f89 100644 --- a/internal/model/mark_model.go +++ b/internal/model/mark_model.go @@ -45,14 +45,29 @@ func (m *markModel) reloadMarkModel() { collection := m.book.form.GetString("collection") mark := m.book.form.GetString("mark") - m.shelf = m.book.shelves.Shelf(shelf) - m.collection = m.book.shelves.Shelf(shelf).Collection(collection) - m.mark = m.book.shelves.Shelf(shelf).Collection(collection).Mark(mark) + s, _ := m.book.shelves.Shelf(shelf) + m.shelf = s + if s == nil { + m.collection = nil + m.mark = nil + return + } + m.collection = s.Collection(collection) + if m.collection == nil { + m.mark = nil + return + } + m.mark = m.collection.Mark(mark) } func (m *markModel) loadMarkParents() { - m.mark.Shelf = m.book.shelves.Shelf(m.book.form.GetString("shelf")) - m.mark.Collection = m.mark.Shelf.Collection(m.book.form.GetString("collection")) + shelf, _ := m.book.shelves.Shelf(m.book.form.GetString("shelf")) + m.mark.Shelf = shelf + if shelf == nil { + m.mark.Collection = nil + return + } + m.mark.Collection = shelf.Collection(m.book.form.GetString("collection")) } func (m *markModel) updateShelfFileCmd(action string) tea.Cmd { @@ -119,7 +134,8 @@ func (m getMarkModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Update model after form has been updated if m.get.book.form.State != huh.StateCompleted { if m.get.book.form.GetString("shelf") != "" { - 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 } if m.get.verifyCollection() { @@ -128,7 +144,7 @@ func (m getMarkModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.get.verifyMark() { // Load valid mark into model for Get|Edit|Delete - if book.StructIsEmpty(m.get.mark) { + if m.get.mark == nil { m.get.reloadMarkModel() } } @@ -238,7 +254,7 @@ func (m getMarkModel) ResultView() string { case "list": return renderCompletedView(s, t, "mark-list", m.get.collection).Content case "delete": - return renderCompletedView(s, t, "mark-delete", m.get.collection).Content + return renderCompletedView(s, t, "mark-delete", m.get.mark).Content } return "" } @@ -274,7 +290,11 @@ func GetMarkForm(bs *book.BookShelves, mark *book.Mark, config *book.Config, act OptionsFunc(func() []huh.Option[string] { // Prevent empty collections from being loaded var opts []string - for _, col := range bs.Shelf(chosenShelf).Collections { + shelf, _ := bs.Shelf(chosenShelf) + if shelf == nil { + return []huh.Option[string]{} + } + for _, col := range shelf.Collections { if (col.HasActiveMarks() && action != "add") || action == "add" { opts = append(opts, col.Name) } @@ -290,14 +310,22 @@ func GetMarkForm(bs *book.BookShelves, mark *book.Mark, config *book.Config, act huh.NewSelect[string](). Title("Pick your mark."). OptionsFunc(func() []huh.Option[string] { - opts := bs.Shelf(chosenShelf).Collection(chosenCollection).MarksNames() + shelf, _ := bs.Shelf(chosenShelf) + if shelf == nil { + return []huh.Option[string]{} + } + collection := shelf.Collection(chosenCollection) + if collection == nil { + return []huh.Option[string]{} + } + opts := collection.MarksNames() return huh.NewOptions(opts...) }, &chosenCollection). Key("mark"). Value(&chosenMark). Height(19), ).WithHideFunc(func() bool { - return !book.StructIsEmpty(mark) || action == "list" + return action == "add" || action == "list" }), huh.NewGroup( @@ -344,7 +372,7 @@ func (m editMarkModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "ctrl+z": switch m.action { case "edit": - getScreen := GetMarkForm(m.editor.book.shelves, &book.Mark{}, m.editor.config, m.action) + getScreen := GetMarkForm(m.editor.book.shelves, nil, m.editor.config, m.action) return getScreen, getScreen.Init() case "add": getScreen := GetMarkForm(m.editor.book.shelves, m.editor.mark, m.editor.config, m.action) @@ -473,7 +501,14 @@ func editMarkForm(bs *book.BookShelves, mark *book.Mark, config *book.Config, ac Title("Review tags."). Description("additional and collection tags shown"). OptionsFunc(func() []huh.Option[string] { - collectTags := bs.Shelf(m.mark.Shelf.Name).Collection(m.mark.Collection.Name).AllTags() + collectTags := []string{} + shelf, _ := bs.Shelf(m.mark.Shelf.Name) + if shelf != nil { + collection := shelf.Collection(m.mark.Collection.Name) + if collection != nil { + collectTags = collection.AllTags() + } + } userTags := book.SplitTagLines(tempTags) return huh.NewOptions(book.MergeTags(m.mark.Tags, userTags, collectTags)...) }, &tempTags). From 8813ed1b789a109753237ff7b7c34b23dd838d14 Mon Sep 17 00:00:00 2001 From: Aaron Martell Date: Wed, 9 Sep 2026 12:32:04 -0500 Subject: [PATCH 2/2] chore: version bump --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 7574079..0d0c52f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v1.3.1 +v1.4.0