From 6ac61ae1c9c9aeb70fe0670f1a76efa4f79b3d36 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 22 Apr 2026 15:19:57 -0400 Subject: [PATCH 1/4] Improve option pattern --- constructors.go | 194 ++++++++++++-------------------- constructors_test.go | 15 +++ extensions/s3/constructor_s3.go | 126 +++++++++------------ option.go | 107 ++++++++++-------- walker.go | 104 +++++++++++++++++ walker_test.go | 127 +++++++++++++++++++++ 6 files changed, 433 insertions(+), 240 deletions(-) create mode 100644 walker.go create mode 100644 walker_test.go diff --git a/constructors.go b/constructors.go index 020c4d6..d66a4f2 100644 --- a/constructors.go +++ b/constructors.go @@ -28,13 +28,13 @@ import ( // functions provided by the other constructors to filter, limit, and // expand the tree during construction. func NewTree[T any](nodes []*Node[T], opts ...Option[T]) *Tree[T] { - cfg := NewMasterConfig(opts) + cfg := newMasterConfig(opts) // Report progress for user-supplied nodes so the callback can still be // leveraged even when callers pre-assemble the slice. if cfg.progressCb != nil { for i, n := range nodes { - cfg.ReportProgress(i+1, n) + cfg.reportProgress(i+1, n) } } @@ -53,11 +53,10 @@ func NewTree[T any](nodes []*Node[T], opts ...Option[T]) *Tree[T] { applyExpansion(nodes, cfg) } - return NewTreeFromCfg(nodes, cfg) + return newTree(nodes, cfg) } -// NewTreeFromCfg creates a new Tree with the provided nodes and the configuration `cfg`. -func NewTreeFromCfg[T any](nodes []*Node[T], cfg *MasterConfig[T]) *Tree[T] { +func newTree[T any](nodes []*Node[T], cfg *masterConfig[T]) *Tree[T] { // Initialize focus to the first node if available var focusedNodes []*Node[T] focusedIDs := make(map[string]bool) @@ -127,7 +126,7 @@ func NewTreeFromNestedData[T any]( provider NestedDataProvider[T], opts ...Option[T], ) (*Tree[T], error) { - cfg := NewMasterConfig(opts) + cfg := newMasterConfig(opts) // Build the node hierarchy using the collected build options. nodes, err := buildTreeFromNestedData(ctx, items, provider, cfg) @@ -137,11 +136,11 @@ func NewTreeFromNestedData[T any]( err = fmt.Errorf("%w: %w", ErrTreeConstruction, err) } - tree := NewTreeFromCfg(nodes, cfg) + tree := newTree(nodes, cfg) return tree, err } -func buildTreeFromNestedData[T any](ctx context.Context, items []T, provider NestedDataProvider[T], cfg *MasterConfig[T]) ([]*Node[T], error) { +func buildTreeFromNestedData[T any](ctx context.Context, items []T, provider NestedDataProvider[T], cfg *masterConfig[T]) ([]*Node[T], error) { nodeCount := 0 hitTraversalCap := false @@ -152,10 +151,10 @@ func buildTreeFromNestedData[T any](ctx context.Context, items []T, provider Nes if err := ctx.Err(); err != nil { return nil, err // Context has ended } - if cfg.ShouldFilter(item) { + if cfg.shouldFilter(item) { return nil, nil // Item was filtered out } - if cfg.HasTraversalCapBeenReached(nodeCount) { + if cfg.hasTraversalCapBeenReached(nodeCount) { hitTraversalCap = true // We've hit the traversal cap return nil, nil } @@ -169,11 +168,11 @@ func buildTreeFromNestedData[T any](ctx context.Context, items []T, provider Nes // Increment node count & report progress nodeCount++ - cfg.ReportProgress(nodeCount, n) + cfg.reportProgress(nodeCount, n) // Check depth limit - if cfg.HasDepthLimitBeenReached(depth) { - cfg.HandleExpansion(n) + if cfg.hasDepthLimitBeenReached(depth) { + cfg.handleExpansion(n) // Return the node without children. return n, nil } @@ -190,7 +189,7 @@ func buildTreeFromNestedData[T any](ctx context.Context, items []T, provider Nes } } - cfg.HandleExpansion(n) + cfg.handleExpansion(n) return n, nil } @@ -261,7 +260,7 @@ func NewTreeFromFlatData[T any]( opts ...Option[T], ) (*Tree[T], error) { // 1. Create config from options. - cfg := NewMasterConfig(opts) + cfg := newMasterConfig(opts) // 2. Build the node hierarchy. nodes, err := buildTreeFromFlatData(ctx, items, provider, cfg) @@ -271,11 +270,11 @@ func NewTreeFromFlatData[T any]( err = fmt.Errorf("%w: %w", ErrTreeConstruction, err) } - tree := NewTreeFromCfg(nodes, cfg) + tree := newTree(nodes, cfg) return tree, err } -func buildTreeFromFlatData[T any](ctx context.Context, items []T, provider FlatDataProvider[T], cfg *MasterConfig[T]) ([]*Node[T], error) { +func buildTreeFromFlatData[T any](ctx context.Context, items []T, provider FlatDataProvider[T], cfg *masterConfig[T]) ([]*Node[T], error) { // Pass 1: Convert all raw items to *Node values, and cache relationship map // parentLookup maps a node ID to the ID of its parent, so we can wire the @@ -292,10 +291,10 @@ func buildTreeFromFlatData[T any](ctx context.Context, items []T, provider FlatD if err := ctx.Err(); err != nil { return nil, err // Context has ended } - if cfg.ShouldFilter(item) { + if cfg.shouldFilter(item) { return nil, nil // Item was filtered out } - if cfg.HasTraversalCapBeenReached(nodeCount) { + if cfg.hasTraversalCapBeenReached(nodeCount) { hitTraversalCap = true // We've hit the traversal cap break } @@ -311,7 +310,7 @@ func buildTreeFromFlatData[T any](ctx context.Context, items []T, provider FlatD parentLookup[id] = provider.ParentID(item) idToNode[id] = n nodeCount++ - cfg.ReportProgress(nodeCount, n) + cfg.reportProgress(nodeCount, n) } // Pass 2: Establish parent/child relationships and validate tree has no cycles @@ -322,7 +321,7 @@ func buildTreeFromFlatData[T any](ctx context.Context, items []T, provider FlatD return nil, err // Context has ended } node := idToNode[id] - cfg.HandleExpansion(node) + cfg.handleExpansion(node) // Gather root notes if parentID == "" { @@ -397,134 +396,87 @@ func NewTreeFromFileSystem( followSymlinks bool, opts ...Option[FileInfo], ) (*Tree[FileInfo], error) { - // 1. Create config with a default provider for the filesystem. - cfg := NewMasterConfig(opts, WithProvider[FileInfo](NewDefaultNodeProvider( + allOpts := append([]Option[FileInfo]{WithProvider[FileInfo](NewDefaultNodeProvider( WithFileExtensionRules[FileInfo](), - ))) - - // 2. Build the node hierarchy from the filesystem. - nodes, err := buildFileSystemTree(ctx, path, followSymlinks, cfg) + ))}, opts...) + tree, err := NewTreeFromWalker(ctx, &fileSystemWalker{ + path: path, + followSymlinks: followSymlinks, + visited: make(map[string]struct{}), + }, allOpts...) if err != nil { - return nil, fmt.Errorf("%w: %w", ErrFileSystem, err) + return tree, wrapFileSystemConstructorError(err) } - - // 3. Create the final tree with a specialized filesystem provider. - tree := NewTreeFromCfg(nodes, cfg) return tree, nil } -func buildFileSystemTree(ctx context.Context, path string, followSymlinks bool, cfg *MasterConfig[FileInfo]) ([]*Node[FileInfo], error) { - // Resolve the path to absolute form, handling `~`, `..`, `.` expansion - absPath, err := utils.ResolvePath(path) - if err != nil { - return nil, pathError(ErrPathResolution, path, err) +func wrapFileSystemConstructorError(err error) error { + if err == nil { + return nil } - - // Track visited inodes to detect symlink loops - // Key is device:inode pair (Unix) or approximation (Windows) - visited := make(map[string]struct{}) - - // Get file info for the root path - // This handles symlinks based on config settings - info, err := utils.SafeStat(absPath, followSymlinks, visited) - if err != nil { - return nil, pathError(ErrFileSystem, absPath, err) + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || errors.Is(err, ErrTraversalLimit) { + return err } - - // Initialize traversal counter - total := 1 - if cfg.HasTraversalCapBeenReached(total) { - return nil, pathError(ErrTraversalLimit, absPath, nil) + if errors.Is(err, ErrFileSystem) { + return err } + return fmt.Errorf("%w: %w", ErrFileSystem, err) +} - // Create the root node - rootNode := NewFileSystemNode(absPath, info) - cfg.ReportProgress(1, rootNode) +type fileSystemWalker struct { + path string + followSymlinks bool + visited map[string]struct{} +} - // Apply initial expansion state if configured - cfg.HandleExpansion(rootNode) +func (w *fileSystemWalker) Root(context.Context) (WalkItem[FileInfo], error) { + absPath, err := utils.ResolvePath(w.path) + if err != nil { + return WalkItem[FileInfo]{}, pathError(ErrPathResolution, w.path, err) + } - // If root is a directory, recursively scan its contents - if info.IsDir() { - if err := scanDir(ctx, rootNode, 0, followSymlinks, cfg, visited, &total); err != nil { - return nil, err - } + info, err := utils.SafeStat(absPath, w.followSymlinks, w.visited) + if err != nil { + return WalkItem[FileInfo]{}, pathError(ErrFileSystem, absPath, err) } - // Return single-element slice containing the root - return []*Node[FileInfo]{rootNode}, nil + return WalkItem[FileInfo]{ + ID: absPath, + Name: info.Name(), + Data: FileInfo{FileInfo: info, Path: absPath}, + }, nil } -// scanDir scans a directory and its subdirectories, creating Node[FileInfo] for each entry. -// It returns an error if the traversal cap is exceeded or if there is an error. -func scanDir(ctx context.Context, parent *Node[FileInfo], depth int, followSymlinks bool, cfg *MasterConfig[FileInfo], visited map[string]struct{}, count *int) error { - // Enforce depth limit if configured - if cfg.HasDepthLimitBeenReached(depth) { - return nil +func (w *fileSystemWalker) Children(ctx context.Context, parent WalkItem[FileInfo]) ([]WalkItem[FileInfo], error) { + if !parent.Data.IsDir() { + return nil, nil } - // Read all entries in the directory - entries, err := os.ReadDir(parent.Data().Path) + entries, err := os.ReadDir(parent.Data.Path) if err != nil { - return pathError(ErrDirectoryScan, parent.Data().Path, err) + return nil, pathError(ErrDirectoryScan, parent.Data.Path, err) } - // Collect child nodes as we process entries - var children []*Node[FileInfo] + children := make([]WalkItem[FileInfo], 0, len(entries)) for _, entry := range entries { - // Check for cancellation between entries if err := ctx.Err(); err != nil { - return err + return nil, err } - // Build full path for the child entry - childPath := filepath.Join(parent.Data().Path, entry.Name()) - - // Get file info, following symlinks if configured - // This also updates the visited map to detect loops - info, err := utils.SafeStat(childPath, followSymlinks, visited) + childPath := filepath.Join(parent.Data.Path, entry.Name()) + info, err := utils.SafeStat(childPath, w.followSymlinks, w.visited) if err != nil { - return pathError(ErrFileSystem, childPath, err) - } - - // Apply filter function if provided - if cfg.ShouldFilter(FileInfo{ - FileInfo: info, - Path: childPath, - }) { - continue // Item was filtered out - } - - // Create node for this entry - childNode := NewFileSystemNode(childPath, info) - - // Apply expansion state if configured - cfg.HandleExpansion(childNode) - - // Increment and check traversal count - // This prevents runaway scans of huge directories - *count++ - cfg.ReportProgress(*count, childNode) - if cfg.HasTraversalCapBeenReached(*count) { - return pathError(ErrTraversalLimit, childPath, nil) + return nil, pathError(ErrFileSystem, childPath, err) } - // Recursively scan subdirectories - if info.IsDir() { - if err := scanDir(ctx, childNode, depth+1, followSymlinks, cfg, visited, count); err != nil { - return err - } - } - - // Add to children list - children = append(children, childNode) + children = append(children, WalkItem[FileInfo]{ + ID: childPath, + Name: info.Name(), + Data: FileInfo{FileInfo: info, Path: childPath}, + }) } - // Attach all children to the parent node - if len(children) > 0 { - parent.SetChildren(children) - } - return nil + return children, nil } // filterNodes recursively filters nodes based on the provided filter function. @@ -586,10 +538,10 @@ func limitDepth[T any](nodes []*Node[T], maxDepth int, currentDepth int) []*Node } // applyExpansion recursively applies the expansion function to all nodes in the tree. -func applyExpansion[T any](nodes []*Node[T], cfg *MasterConfig[T]) { +func applyExpansion[T any](nodes []*Node[T], cfg *masterConfig[T]) { for _, node := range nodes { // Apply expansion function to this node - cfg.HandleExpansion(node) + cfg.handleExpansion(node) // Recursively apply to children applyExpansion(node.Children(), cfg) diff --git a/constructors_test.go b/constructors_test.go index df671f3..1ad70fe 100644 --- a/constructors_test.go +++ b/constructors_test.go @@ -755,6 +755,18 @@ func TestNewTreeFromFileSystem(t *testing.T) { WithTraversalCap[FileInfo](3), }, wantErr: true, + checkFn: func(t *testing.T, tree *Tree[FileInfo]) { + t.Helper() + if tree == nil { + t.Fatal("NewTreeFromFileSystem(traversal cap) tree = nil, want partial tree") + } + if len(tree.nodes) != 1 { + t.Fatalf("NewTreeFromFileSystem(traversal cap) root count = %d, want 1", len(tree.nodes)) + } + if len(tree.nodes[0].Children()) == 0 { + t.Fatal("NewTreeFromFileSystem(traversal cap) root has no partial children") + } + }, }, { name: "with_progress_callback", @@ -779,6 +791,9 @@ func TestNewTreeFromFileSystem(t *testing.T) { if err == nil { t.Errorf("NewTreeFromFileSystem(%s) error = nil, want error", test.name) } + if test.checkFn != nil { + test.checkFn(t, got) + } return } diff --git a/extensions/s3/constructor_s3.go b/extensions/s3/constructor_s3.go index 1558d02..3c1635e 100644 --- a/extensions/s3/constructor_s3.go +++ b/extensions/s3/constructor_s3.go @@ -3,19 +3,13 @@ package s3 import ( "context" + "errors" "fmt" - "sync" - "sync/atomic" "github.com/Digital-Shane/treeview" - "github.com/Digital-Shane/treeview/extensions/s3/internal/s3" + internals3 "github.com/Digital-Shane/treeview/extensions/s3/internal/s3" ) -type safeThreadNode struct { - *treeview.Node[treeview.FileInfo] - sync.RWMutex -} - // NewTreeFromS3 creates a new tree structure based on files fetched from an S3 path, using configurable options. // Returns a pointer to a Tree structure or an error if an issue occurs during tree creation. // @@ -25,92 +19,82 @@ type safeThreadNode struct { // - treeview.WithMaxDepth: Limits tree depth during construction // - treeview.WithExpandFunc: Sets initial expansion state for nodes // - treeview.WithTraversalCap: Limits total nodes processed (returns a partial tree + error if exceeded) -// - treeview.WithProgressCallback: Invoked after each filesystem entry is processed (breadth-first per directory) +// - treeview.WithProgressCallback: Invoked after each node is created during traversal func NewTreeFromS3(ctx context.Context, path string, profile string, opts ...treeview.Option[treeview.FileInfo]) (*treeview.Tree[treeview.FileInfo], error) { - cfg := treeview.NewMasterConfig(opts, treeview.WithProvider[treeview.FileInfo](treeview.NewDefaultNodeProvider( + allOpts := append([]treeview.Option[treeview.FileInfo]{treeview.WithProvider[treeview.FileInfo](treeview.NewDefaultNodeProvider( treeview.WithFileExtensionRules[treeview.FileInfo](), - ))) - nodes, err := buildFileSystemTreeForS3(ctx, path, profile, cfg) + ))}, opts...) + + tree, err := treeview.NewTreeFromWalker(ctx, &walker{path: path, profile: profile}, allOpts...) if err != nil { - return nil, fmt.Errorf("%w: %w", treeview.ErrFileSystem, err) + return tree, wrapConstructorError(err) } - tree := treeview.NewTreeFromCfg(nodes, cfg) + return tree, nil } -func buildFileSystemTreeForS3(ctx context.Context, path string, profile string, - cfg *treeview.MasterConfig[treeview.FileInfo]) ([]*treeview.Node[treeview.FileInfo], error) { - info, err := s3.Info(ctx, path, s3.WithProfile(profile)) - if err != nil { - return nil, pathError(treeview.ErrPathResolution, path, err) +func wrapConstructorError(err error) error { + if err == nil { + return nil + } + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || errors.Is(err, treeview.ErrTraversalLimit) { + return err } - total := int64(1) + if errors.Is(err, treeview.ErrFileSystem) { + return err + } + return fmt.Errorf("%w: %w", treeview.ErrFileSystem, err) +} - rootNode := safeThreadNode{Node: treeview.NewFileSystemNode(path, info), RWMutex: sync.RWMutex{}} - cfg.HandleExpansion(rootNode.Node) - if info.IsDir() { - if err := scanDirS3(ctx, &rootNode, 0, cfg, &total); err != nil { - return nil, err - } +type walker struct { + path string + profile string +} + +func (w *walker) Root(ctx context.Context) (treeview.WalkItem[treeview.FileInfo], error) { + info, err := internals3.Info(ctx, w.path, internals3.WithProfile(w.profile)) + if err != nil { + return treeview.WalkItem[treeview.FileInfo]{}, pathError(treeview.ErrPathResolution, w.path, err) } - return []*treeview.Node[treeview.FileInfo]{rootNode.Node}, nil + + return treeview.WalkItem[treeview.FileInfo]{ + ID: w.path, + Name: info.Name(), + Data: treeview.FileInfo{FileInfo: info, Path: w.path}, + }, nil } -// scanDirS3 scans a bucket or key and its subdirectories, creating Node[treeview.FileInfo] for each entry. -// It returns an error if the traversal cap is exceeded or if there is an error. -func scanDirS3(ctx context.Context, parent *safeThreadNode, depth int, cfg *treeview.MasterConfig[treeview.FileInfo], - count *int64) error { - if cfg.HasDepthLimitBeenReached(depth) { - return nil +func (w *walker) Children(ctx context.Context, parent treeview.WalkItem[treeview.FileInfo]) ([]treeview.WalkItem[treeview.FileInfo], error) { + if !parent.Data.IsDir() { + return nil, nil } - parent.RLock() - p := parent.Data().Path - parent.RUnlock() - entries, err := s3.ReadDir(ctx, p) + + entries, err := internals3.ReadDir(ctx, parent.Data.Path) if err != nil { - return pathError(treeview.ErrDirectoryScan, p, err) + return nil, pathError(treeview.ErrDirectoryScan, parent.Data.Path, err) } - children := make([]*treeview.Node[treeview.FileInfo], 0, len(entries)) // preallocation of the capacity only. + + children := make([]treeview.WalkItem[treeview.FileInfo], 0, len(entries)) for _, entry := range entries { - // Check for cancellation between entries if err := ctx.Err(); err != nil { - return err + return nil, err } - childPath := s3.Join(p, entry.Name()) // entry.Name is the full key. + + childPath := internals3.Join(parent.Data.Path, entry.Name()) info, err := entry.Info() if err != nil { - return pathError(treeview.ErrFileSystem, childPath, err) + return nil, pathError(treeview.ErrFileSystem, childPath, err) } - if cfg.ShouldFilter(treeview.FileInfo{ - FileInfo: info, - Path: childPath, - }) { - continue // Item was filtered out - } - childNode := safeThreadNode{Node: treeview.NewFileSystemNode(childPath, info), RWMutex: sync.RWMutex{}} - cfg.HandleExpansion(childNode.Node) - atomic.AddInt64(count, 1) - val := int(atomic.LoadInt64(count)) - cfg.ReportProgress(val, childNode.Node) - if cfg.HasTraversalCapBeenReached(val) { - return pathError(treeview.ErrTraversalLimit, childPath, nil) - } - if info.IsDir() { - if err := scanDirS3(ctx, &childNode, depth+1, cfg, count); err != nil { - return err - } - } - childNode.RLock() - children = append(children, childNode.Node) - childNode.RUnlock() - } - if len(children) > 0 { - parent.Lock() - parent.SetChildren(children) - parent.Unlock() + + children = append(children, treeview.WalkItem[treeview.FileInfo]{ + ID: childPath, + Name: info.Name(), + Data: treeview.FileInfo{FileInfo: info, Path: childPath}, + }) } - return nil + + return children, nil } // pathError creates an error that includes path context. diff --git a/option.go b/option.go index 208948c..3f22f27 100644 --- a/option.go +++ b/option.go @@ -22,71 +22,83 @@ type ProgressCallback[T any] func(processed int, node *Node[T]) // list. The offset is usually ±1 but can be any integer. type FocusPolicyFn[T any] func(ctx context.Context, visible []*Node[T], current *Node[T], offset int) (*Node[T], error) -// Option is the unified functional Option type used by all tree constructors. -// It allows callers to provide build-time and run-time configurations in a -// single, flat list. -type Option[T any] func(*MasterConfig[T]) +// Option is the unified option type used by all tree constructors. +// +// It allows callers to provide build-time and run-time configuration in a +// single, flat list while preventing external packages from constructing their +// own option implementations. Options are sealed to the treeview package; use +// the exported With... helpers to construct them. +type Option[T any] interface { + apply(*masterConfig[T]) +} + +type optionFunc[T any] func(*masterConfig[T]) + +func (fn optionFunc[T]) apply(cfg *masterConfig[T]) { + fn(cfg) +} // WithProvider specifies the NodeProvider to use for rendering nodes. // The provider controls how nodes are formatted, styled, and displayed. func WithProvider[T any](p NodeProvider[T]) Option[T] { - return func(cfg *MasterConfig[T]) { + return optionFunc[T](func(cfg *masterConfig[T]) { cfg.provider = p - } + }) } // WithSearcher overwrites the algorithm used when the search feature is invoked. func WithSearcher[T any](fn SearchFn[T]) Option[T] { - return func(cfg *MasterConfig[T]) { + return optionFunc[T](func(cfg *masterConfig[T]) { cfg.searcher = fn - } + }) } // WithFocusPolicy replaces the logic that decides which node should be focused // after search or navigation. func WithFocusPolicy[T any](fn FocusPolicyFn[T]) Option[T] { - return func(cfg *MasterConfig[T]) { + return optionFunc[T](func(cfg *masterConfig[T]) { cfg.focusPol = fn - } + }) } // WithExpandFunc installs a predicate that decides for each node whether it // should start expanded. func WithExpandFunc[T any](fn ExpandFn[T]) Option[T] { - return func(cfg *MasterConfig[T]) { + return optionFunc[T](func(cfg *masterConfig[T]) { cfg.expandFunc = fn - } + }) } // WithExpandAll expands all nodes by default. func WithExpandAll[T any]() Option[T] { - return func(cfg *MasterConfig[T]) { + return optionFunc[T](func(cfg *masterConfig[T]) { cfg.expandFunc = func(*Node[T]) bool { return true } - } + }) } // WithFilterFunc installs a predicate that decides for each node whether it // should be included in the tree. func WithFilterFunc[T any](filter FilterFn[T]) Option[T] { - return func(cfg *MasterConfig[T]) { + return optionFunc[T](func(cfg *masterConfig[T]) { cfg.filterFunc = filter - } + }) } -// WithMaxDepth limits how deep the walker descends into directories or other -// data structures. Use a negative depth for no limit (default). +// WithMaxDepth limits how deep the constructors descend. +// A depth of 0 keeps only root nodes, 1 includes roots plus their children, and +// a negative depth means no limit (default). func WithMaxDepth[T any](d int) Option[T] { - return func(c *MasterConfig[T]) { + return optionFunc[T](func(c *masterConfig[T]) { c.maxDepth = d - } + }) } -// WithTraversalCap sets an upper bound on the total number of entries visited +// WithTraversalCap sets an upper bound on the total number of nodes created // during tree construction. A value ≤ 0 is interpreted as no limit. func WithTraversalCap[T any](cap int) Option[T] { - return func(c *MasterConfig[T]) { + return optionFunc[T](func(c *masterConfig[T]) { c.traversalCap = cap - } + }) } // WithProgressCallback registers a callback that is invoked each time a new @@ -94,25 +106,23 @@ func WithTraversalCap[T any](cap int) Option[T] { // invoked by NewTree (which accepts pre-built nodes) except once per root // node supplied so callers have a consistent hook. func WithProgressCallback[T any](cb ProgressCallback[T]) Option[T] { - return func(c *MasterConfig[T]) { + return optionFunc[T](func(c *masterConfig[T]) { c.progressCb = cb - } + }) } // WithTruncate sets the maximum width for rendered lines. Lines longer than // this width will be truncated with an ellipsis. A width of 0 disables // truncation (default). func WithTruncate[T any](width int) Option[T] { - return func(c *MasterConfig[T]) { + return optionFunc[T](func(c *masterConfig[T]) { c.truncateWidth = width - } + }) } -// MasterConfig is the structure that aggregates options from -// different domains (build, filesystem, tree). It is used by the unified -// constructors to collect and dispatch options to the appropriate internal -// functions. -type MasterConfig[T any] struct { +// masterConfig aggregates options from different domains (build, filesystem, +// tree) for use by the internal constructors. +type masterConfig[T any] struct { // Options used during the build process. maxDepth int traversalCap int @@ -127,9 +137,10 @@ type MasterConfig[T any] struct { truncateWidth int // Maximum width for rendered lines (0 = no truncation) } -// NewMasterConfig is a helper that creates a MasterConfig, applies defaults, and then user-provided options. -func NewMasterConfig[T any](opts []Option[T], defaults ...Option[T]) *MasterConfig[T] { - cfg := &MasterConfig[T]{ +// newMasterConfig creates a config, applies defaults, and then user-provided +// options. +func newMasterConfig[T any](opts []Option[T], defaults ...Option[T]) *masterConfig[T] { + cfg := &masterConfig[T]{ searcher: defaultSearchFn[T], focusPol: defaultFocusPolicy[T], provider: NewDefaultNodeProvider[T](), @@ -143,30 +154,30 @@ func NewMasterConfig[T any](opts []Option[T], defaults ...Option[T]) *MasterConf // Apply provided defaults first for _, opt := range defaults { if opt != nil { - opt(cfg) + opt.apply(cfg) } } // Apply user-provided options. for _, opt := range opts { if opt != nil { - opt(cfg) + opt.apply(cfg) } } return cfg } -// ShouldFilter evaluates whether the given item should be excluded based on the configured filter function. -func (cfg *MasterConfig[T]) ShouldFilter(item T) bool { +// shouldFilter evaluates whether the given item should be excluded based on the configured filter function. +func (cfg *masterConfig[T]) shouldFilter(item T) bool { if cfg.filterFunc == nil { return false } return !cfg.filterFunc(item) } -// HandleExpansion checks if a node should be expanded based on the configured expand function and expands it if true. -func (cfg *MasterConfig[T]) HandleExpansion(node *Node[T]) { +// handleExpansion checks if a node should be expanded based on the configured expand function and expands it if true. +func (cfg *masterConfig[T]) handleExpansion(node *Node[T]) { if cfg.expandFunc == nil { return } @@ -175,24 +186,24 @@ func (cfg *MasterConfig[T]) HandleExpansion(node *Node[T]) { } } -// HasTraversalCapBeenReached checks if the current node count has reached or exceeded the configured traversal cap. -func (cfg *MasterConfig[T]) HasTraversalCapBeenReached(nodeCount int) bool { +// hasTraversalCapBeenReached checks if the current node count has reached or exceeded the configured traversal cap. +func (cfg *masterConfig[T]) hasTraversalCapBeenReached(nodeCount int) bool { if cfg.traversalCap <= 0 { return false } return nodeCount >= cfg.traversalCap } -// HasDepthLimitBeenReached checks if the given current depth has reached or exceeded the configured maximum depth limit. -func (cfg *MasterConfig[T]) HasDepthLimitBeenReached(currentDepth int) bool { - if cfg.maxDepth <= 0 { +// hasDepthLimitBeenReached checks if the given current depth has reached or exceeded the configured maximum depth limit. +func (cfg *masterConfig[T]) hasDepthLimitBeenReached(currentDepth int) bool { + if cfg.maxDepth < 0 { return false } return currentDepth >= cfg.maxDepth } -// ReportProgress invokes the configured progress callback (if any). -func (cfg *MasterConfig[T]) ReportProgress(processed int, node *Node[T]) { +// reportProgress invokes the configured progress callback (if any). +func (cfg *masterConfig[T]) reportProgress(processed int, node *Node[T]) { if cfg.progressCb == nil || node == nil { return } diff --git a/walker.go b/walker.go new file mode 100644 index 0000000..b1eeace --- /dev/null +++ b/walker.go @@ -0,0 +1,104 @@ +package treeview + +import "context" + +// WalkItem describes a single source item that can be converted into a tree node. +type WalkItem[T any] struct { + ID string + Name string + Data T +} + +// Walker provides a source-specific traversal adapter for NewTreeFromWalker. +// +// Implementations provide the root item and enumerate children for any given +// parent. The treeview package owns filtering, depth limiting, traversal caps, +// expansion, progress callbacks, and parent/child wiring. Filtering is applied +// before descent, so filtered parents are pruned with their descendants. +type Walker[T any] interface { + Root(context.Context) (WalkItem[T], error) + Children(context.Context, WalkItem[T]) ([]WalkItem[T], error) +} + +// NewTreeFromWalker builds a tree from a Walker. +// +// Walker errors are returned as-is. Context errors are returned unwrapped. +func NewTreeFromWalker[T any](ctx context.Context, walker Walker[T], opts ...Option[T]) (*Tree[T], error) { + cfg := newMasterConfig(opts) + nodes, err := buildTreeFromWalker(ctx, walker, cfg) + return newTree(nodes, cfg), err +} + +func buildTreeFromWalker[T any](ctx context.Context, walker Walker[T], cfg *masterConfig[T]) ([]*Node[T], error) { + rootItem, err := walker.Root(ctx) + if err != nil { + return nil, err + } + + nodeCount := 0 + hitTraversalCap := false + + var buildSubtree func(WalkItem[T], int) (*Node[T], error) + buildSubtree = func(item WalkItem[T], depth int) (*Node[T], error) { + if err := ctx.Err(); err != nil { + return nil, err + } + if cfg.shouldFilter(item.Data) { + return nil, nil + } + if cfg.hasTraversalCapBeenReached(nodeCount) { + hitTraversalCap = true + return nil, nil + } + if item.ID == "" { + return nil, ErrEmptyID + } + + node := NewNode(item.ID, item.Name, item.Data) + nodeCount++ + cfg.reportProgress(nodeCount, node) + cfg.handleExpansion(node) + + if cfg.hasDepthLimitBeenReached(depth) { + return node, nil + } + + children, err := walker.Children(ctx, item) + if err != nil { + return nil, err + } + + childNodes := make([]*Node[T], 0, len(children)) + for _, child := range children { + childNode, err := buildSubtree(child, depth+1) + if err != nil { + return nil, err + } + if childNode != nil { + childNodes = append(childNodes, childNode) + } + if hitTraversalCap { + break + } + } + + if len(childNodes) > 0 { + node.SetChildren(childNodes) + } + + return node, nil + } + + rootNode, err := buildSubtree(rootItem, 0) + if err != nil { + return nil, err + } + if rootNode == nil { + return nil, nil + } + if hitTraversalCap { + return []*Node[T]{rootNode}, ErrTraversalLimit + } + + return []*Node[T]{rootNode}, nil +} diff --git a/walker_test.go b/walker_test.go new file mode 100644 index 0000000..577e675 --- /dev/null +++ b/walker_test.go @@ -0,0 +1,127 @@ +package treeview_test + +import ( + "context" + "errors" + "testing" + + "github.com/Digital-Shane/treeview" +) + +type fixtureWalker struct { + root treeview.WalkItem[string] + children map[string][]treeview.WalkItem[string] +} + +func (w *fixtureWalker) Root(context.Context) (treeview.WalkItem[string], error) { + return w.root, nil +} + +func (w *fixtureWalker) Children(_ context.Context, parent treeview.WalkItem[string]) ([]treeview.WalkItem[string], error) { + return w.children[parent.ID], nil +} + +func TestNewTreeFromWalker(t *testing.T) { + ctx := context.Background() + walker := &fixtureWalker{ + root: treeview.WalkItem[string]{ID: "root", Name: "Root", Data: "root"}, + children: map[string][]treeview.WalkItem[string]{ + "root": { + {ID: "keep", Name: "Keep", Data: "keep"}, + {ID: "skip", Name: "Skip", Data: "skip"}, + }, + "keep": { + {ID: "grandchild", Name: "Grandchild", Data: "grandchild"}, + }, + }, + } + + progressCalls := 0 + tree, err := treeview.NewTreeFromWalker(ctx, walker, + treeview.WithFilterFunc(func(item string) bool { return item != "skip" }), + treeview.WithMaxDepth[string](1), + treeview.WithExpandFunc[string](func(node *treeview.Node[string]) bool { + return node.ID() == "root" + }), + treeview.WithProgressCallback[string](func(processed int, node *treeview.Node[string]) { + if processed <= 0 || node == nil { + t.Fatalf("progress callback received invalid values: processed=%d node=%v", processed, node) + } + progressCalls++ + }), + ) + if err != nil { + t.Fatalf("NewTreeFromWalker() error = %v, want nil", err) + } + + if len(tree.Nodes()) != 1 { + t.Fatalf("NewTreeFromWalker() root count = %d, want 1", len(tree.Nodes())) + } + + root := tree.Nodes()[0] + if !root.IsExpanded() { + t.Fatal("NewTreeFromWalker() root is not expanded") + } + if len(root.Children()) != 1 { + t.Fatalf("NewTreeFromWalker() root children = %d, want 1", len(root.Children())) + } + if got := root.Children()[0].ID(); got != "keep" { + t.Fatalf("NewTreeFromWalker() child ID = %q, want %q", got, "keep") + } + if len(root.Children()[0].Children()) != 0 { + t.Fatalf("NewTreeFromWalker() child grandchildren = %d, want 0 due to max depth", len(root.Children()[0].Children())) + } + if progressCalls != 2 { + t.Fatalf("NewTreeFromWalker() progress calls = %d, want 2", progressCalls) + } +} + +func TestNewTreeFromWalkerMaxDepthZero(t *testing.T) { + ctx := context.Background() + walker := &fixtureWalker{ + root: treeview.WalkItem[string]{ID: "root", Name: "Root", Data: "root"}, + children: map[string][]treeview.WalkItem[string]{ + "root": { + {ID: "child", Name: "Child", Data: "child"}, + }, + }, + } + + tree, err := treeview.NewTreeFromWalker(ctx, walker, treeview.WithMaxDepth[string](0)) + if err != nil { + t.Fatalf("NewTreeFromWalker() error = %v, want nil", err) + } + if len(tree.Nodes()) != 1 { + t.Fatalf("NewTreeFromWalker() root count = %d, want 1", len(tree.Nodes())) + } + if len(tree.Nodes()[0].Children()) != 0 { + t.Fatalf("NewTreeFromWalker() root children = %d, want 0 with max depth 0", len(tree.Nodes()[0].Children())) + } +} + +func TestNewTreeFromWalkerTraversalCap(t *testing.T) { + ctx := context.Background() + walker := &fixtureWalker{ + root: treeview.WalkItem[string]{ID: "root", Name: "Root", Data: "root"}, + children: map[string][]treeview.WalkItem[string]{ + "root": { + {ID: "a", Name: "A", Data: "a"}, + {ID: "b", Name: "B", Data: "b"}, + }, + }, + } + + tree, err := treeview.NewTreeFromWalker(ctx, walker, treeview.WithTraversalCap[string](2)) + if !errors.Is(err, treeview.ErrTraversalLimit) { + t.Fatalf("NewTreeFromWalker() error = %v, want ErrTraversalLimit", err) + } + if tree == nil { + t.Fatal("NewTreeFromWalker() tree = nil, want partial tree") + } + if len(tree.Nodes()) != 1 { + t.Fatalf("NewTreeFromWalker() root count = %d, want 1", len(tree.Nodes())) + } + if len(tree.Nodes()[0].Children()) != 1 { + t.Fatalf("NewTreeFromWalker() partial child count = %d, want 1", len(tree.Nodes()[0].Children())) + } +} From f5363c8a4dd3003c58249e5344f01496826f946d Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 22 Apr 2026 15:29:41 -0400 Subject: [PATCH 2/4] Fix the unused apply function issue goci found --- option.go | 54 ++++++++++++++++++++++++------------------------------ 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/option.go b/option.go index 3f22f27..015d74e 100644 --- a/option.go +++ b/option.go @@ -28,77 +28,71 @@ type FocusPolicyFn[T any] func(ctx context.Context, visible []*Node[T], current // single, flat list while preventing external packages from constructing their // own option implementations. Options are sealed to the treeview package; use // the exported With... helpers to construct them. -type Option[T any] interface { - apply(*masterConfig[T]) -} - -type optionFunc[T any] func(*masterConfig[T]) - -func (fn optionFunc[T]) apply(cfg *masterConfig[T]) { - fn(cfg) +type Option[T any] struct { + apply func(*masterConfig[T]) } // WithProvider specifies the NodeProvider to use for rendering nodes. // The provider controls how nodes are formatted, styled, and displayed. func WithProvider[T any](p NodeProvider[T]) Option[T] { - return optionFunc[T](func(cfg *masterConfig[T]) { + return Option[T]{apply: func(cfg *masterConfig[T]) { cfg.provider = p - }) + }} } // WithSearcher overwrites the algorithm used when the search feature is invoked. func WithSearcher[T any](fn SearchFn[T]) Option[T] { - return optionFunc[T](func(cfg *masterConfig[T]) { + return Option[T]{apply: func(cfg *masterConfig[T]) { cfg.searcher = fn - }) + }} } // WithFocusPolicy replaces the logic that decides which node should be focused // after search or navigation. func WithFocusPolicy[T any](fn FocusPolicyFn[T]) Option[T] { - return optionFunc[T](func(cfg *masterConfig[T]) { + return Option[T]{apply: func(cfg *masterConfig[T]) { cfg.focusPol = fn - }) + }} } // WithExpandFunc installs a predicate that decides for each node whether it // should start expanded. func WithExpandFunc[T any](fn ExpandFn[T]) Option[T] { - return optionFunc[T](func(cfg *masterConfig[T]) { + return Option[T]{apply: func(cfg *masterConfig[T]) { cfg.expandFunc = fn - }) + }} } // WithExpandAll expands all nodes by default. func WithExpandAll[T any]() Option[T] { - return optionFunc[T](func(cfg *masterConfig[T]) { + return Option[T]{apply: func(cfg *masterConfig[T]) { cfg.expandFunc = func(*Node[T]) bool { return true } - }) + }} } // WithFilterFunc installs a predicate that decides for each node whether it // should be included in the tree. func WithFilterFunc[T any](filter FilterFn[T]) Option[T] { - return optionFunc[T](func(cfg *masterConfig[T]) { + return Option[T]{apply: func(cfg *masterConfig[T]) { cfg.filterFunc = filter - }) + }} } // WithMaxDepth limits how deep the constructors descend. // A depth of 0 keeps only root nodes, 1 includes roots plus their children, and // a negative depth means no limit (default). func WithMaxDepth[T any](d int) Option[T] { - return optionFunc[T](func(c *masterConfig[T]) { + return Option[T]{apply: func(c *masterConfig[T]) { c.maxDepth = d - }) + }} } // WithTraversalCap sets an upper bound on the total number of nodes created // during tree construction. A value ≤ 0 is interpreted as no limit. func WithTraversalCap[T any](cap int) Option[T] { - return optionFunc[T](func(c *masterConfig[T]) { + return Option[T]{apply: func(c *masterConfig[T]) { c.traversalCap = cap - }) + }} } // WithProgressCallback registers a callback that is invoked each time a new @@ -106,18 +100,18 @@ func WithTraversalCap[T any](cap int) Option[T] { // invoked by NewTree (which accepts pre-built nodes) except once per root // node supplied so callers have a consistent hook. func WithProgressCallback[T any](cb ProgressCallback[T]) Option[T] { - return optionFunc[T](func(c *masterConfig[T]) { + return Option[T]{apply: func(c *masterConfig[T]) { c.progressCb = cb - }) + }} } // WithTruncate sets the maximum width for rendered lines. Lines longer than // this width will be truncated with an ellipsis. A width of 0 disables // truncation (default). func WithTruncate[T any](width int) Option[T] { - return optionFunc[T](func(c *masterConfig[T]) { + return Option[T]{apply: func(c *masterConfig[T]) { c.truncateWidth = width - }) + }} } // masterConfig aggregates options from different domains (build, filesystem, @@ -153,14 +147,14 @@ func newMasterConfig[T any](opts []Option[T], defaults ...Option[T]) *masterConf // Apply provided defaults first for _, opt := range defaults { - if opt != nil { + if opt.apply != nil { opt.apply(cfg) } } // Apply user-provided options. for _, opt := range opts { - if opt != nil { + if opt.apply != nil { opt.apply(cfg) } } From 89f001319ce45f80f0f3f82dab177c69e562bf94 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 22 Apr 2026 18:42:03 -0400 Subject: [PATCH 3/4] changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a2f09e..4e7395c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Updated - Updated charm ecosystem libraries to v2. (darkhz) - golangci version to be go 1.25 compatible. +- Reworked extension support around `NewTreeFromWalker` and returned root config internals to private scope. +- Updated `Option` to keep extension support without exposing internal config types. ### Added - Golang CI lint config and workflow. - `WithTuiAltScreen` option to restore old alt screen behavior. +- `Walker`, `WalkItem`, and `NewTreeFromWalker` for extension-backed tree construction. ### Fixed - Fix viewport autoscroll silently clamping to top because content length was set after the scroll offset. From b96d3eeb0d7f03a63af7f95ea28b5b10841976f5 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 22 Apr 2026 21:00:35 -0400 Subject: [PATCH 4/4] Add v2 update info --- CHANGELOG.md | 3 ++ README.md | 3 ++ v2_updates.md | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 v2_updates.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e7395c..e53a7be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ## [v2.0.0] - 2026-04-22 + +See the [v2 update guide](./v2_updates.md) for upgrade notes and migration advice. + ### Updated - Updated charm ecosystem libraries to v2. (darkhz) - golangci version to be go 1.25 compatible. diff --git a/README.md b/README.md index 0dc7739..a84ab98 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,9 @@ **TreeView** is a feature-rich Go library for displaying and navigating tree structures in the terminal. TreeView has full [Bubble Tea][bt] and [Lipgloss][lg] support, allowing you to build glamorous, interactive terminal applications. +If you're upgrading from v1, the [v2 update guide](./v2_updates.md) walks +through the important changes and what to watch for. + ## Why TreeView? What sets TreeView apart is its flexibility. You can build trees from any data source: flat lists with parent relationships, diff --git a/v2_updates.md b/v2_updates.md new file mode 100644 index 0000000..192a41e --- /dev/null +++ b/v2_updates.md @@ -0,0 +1,97 @@ +# Updating to TreeView v2 + +TreeView v2 is mostly a cleanup release, but there are a few changes worth +paying attention to before you bump the version in a real project. If you use +the standard constructors and the exported `With...` helpers, the upgrade will +probably be pretty small. If you built a custom extension or relied on internal +construction details from the root package, this is the release where you will +need to touch code. + +The short version is that v2 moves TreeView onto the Charm v2 ecosystem (Thanks darkhz!), +adds a small compatibility option for TUI apps, cleans up the extension story +around a new walker-based API, and fixes a viewport scrolling bug that some +apps may have worked around manually. + +## Charm v2 + +TreeView now uses the Charm v2 packages. If your project imports Bubble Tea, +Bubbles, or Lip Gloss directly alongside TreeView, you should update those +imports and versions at the same time so everything is on the same stack. + +```go +import tea "charm.land/bubbletea/v2" +import "charm.land/bubbles/v2/viewport" +import "charm.land/lipgloss/v2" +``` + +If you only use TreeView as a library and do not import the Charm packages +yourself, this part may not require much more than updating your dependency and +making sure your local toolchain and CI are running Go 1.25. + +## TUI behavior and `WithTuiAltScreen` + +v2 adds `WithTuiAltScreen(true)` to restore the older alt screen behavior. If +your TUI felt right before and still feels right after the upgrade, you can +ignore this completely. If your app suddenly has the terminal command used to +launch your app persisting at the top add: + +```go +model := treeview.NewTuiTreeModel( + tree, + treeview.WithTuiAltScreen(true), +) +``` + +## The extension system + +This is the biggest API change in v2. Earlier extension support worked by +exposing a few things from the root package that were really supposed to stay +internal. That made the S3 extension possible, but it also meant external code +could depend on internal tree construction details. It worked, but it was not a +good long-term pattern. + +v2 fixes that by introducing a real extension seam built around `Walker`, +`WalkItem`, and `NewTreeFromWalker`. Instead of recreating tree construction in +an extension, you now provide a root item and the children for a given item, and +TreeView handles the shared behavior like filtering, max depth, traversal caps, +expansion, progress callbacks, and final tree assembly. + +```go +type Walker[T any] interface { + Root(context.Context) (treeview.WalkItem[T], error) + Children(context.Context, treeview.WalkItem[T]) ([]treeview.WalkItem[T], error) +} +``` + +If you were using internal root types like `MasterConfig`, `NewMasterConfig`, +or `NewTreeFromCfg`, the important change is those path is gone. The new +path is `NewTreeFromWalker`. + +One practical note here: walker-based filtering happens before descent. If a +parent item is filtered out, its descendants are pruned with it. For most data +sources that is exactly what you want, but it is worth knowing if your older +extension logic assumed it could still descend through filtered parents. + +## `Option` still works, but the internals do not leak anymore + +`Option` is still the public way to configure TreeView constructors, so normal +app code using `WithProvider`, `WithFilterFunc`, `WithExpandFunc`, +`WithMaxDepth`, `WithTraversalCap`, or `WithProgressCallback` should feel the +same. What changed is that extension support no longer depends on exposing the +internal config types that power those options. + +In other words, if you were just passing TreeView options around, you are fine. +If you were building extension code that depended on root-package config +internals, move that logic into your walker or constructor and keep TreeView +configuration on the public `With...` side. + +The filesystem constructor was updated to use the same walker-based path that +extensions use, and the S3 extension was moved over to that model too. + +## Viewport autoscroll was fixed + +v2 also fixes a viewport autoscroll issue where the scroll position could clamp +back to the top because the content length was being set after the scroll +offset. If you added a workaround for odd viewport jumps or the tree snapping back to +the top during navigation, test your app without that workaround first. There is +a good chance you can delete code now.