Improve option pattern#21
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR cleans up a mistake I made when I first approved extension support.
At the time, the quickest way to get the S3 extension working was to expose a few pieces of the root package that were really supposed to stay internal. That included things like the config type and some of the internal tree construction helpers. It worked, but it also meant extensions could depend on implementation details from the core package, which is not a great place to end up. It made the public API bigger than it needed to be, and it made future refactors a lot riskier because internal changes could accidentally turn into breaking API changes.
This change fixes that by introducing a real extension pattern instead of relying on leaked internals. The new path is built around
Walker,WalkItem, andNewTreeFromWalker. Instead of reaching into the root package to recreate tree construction logic, an extension now just needs to describe how to get its root item and how to fetch children for a given item. The core package takes care of the rest, including filtering, max depth, traversal caps, expansion, progress callbacks, and final tree assembly.I also updated the filesystem constructor and the S3 extension to use that same walker-based path. That let me move the internal config and builder types back to private scope again, which was the main goal here.
Optionstill works for extensions, but it no longer depends on exposing internal config machinery just to make that possible.So the big picture is pretty simple: extensions still have a clean way to plug into tree construction, but now they do it through a public API that was actually designed for extensions instead of depending on internals that happened to be exposed to make one use case work.