-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add an API to parse a selector from a string #189
Comments
I do not have a strong opinion on what this should look like, though I want to caution that expressing a multi-match selector ( unions, etc ) as a flat-string is perilous, and should be approached with a lot of care. On the "just express a singular path" front, it would be nice (but not required) to incorporate the rudimentary grammar of https://pkg.go.dev/github.com/ipld/go-ipld-selector-text-lite#SelectorSpecFromPath as a subset. I am open to changing some of the "spec" there before it becomes part of lotus-proper, but changing after ~3 weeks will be a challenge, and could lead to a UX "soft-fork". |
I personally think it would be fine for the flat-string parsing to be limited by nature. If one needs complex tree-like selectors, a linear-looking string isn't going to be good UX anyway. |
I think this may have been addressed by the introduction of easy-to-use parser methods in #199 . (Since then, I also have had some ideas brewing about how I'd like to rearrange the selector package and its children for better discoverability, but that's another matter.) I'm going to call this solved and close this -- if we are interested in further APIs for stringy selectors that are not just data model encodings, I would also suggest we open those as design research issues in the |
The "builder" API is powerful, but tricky to use for simple use cases: https://pkg.go.dev/github.com/ipld/go-ipld-prime/traversal/selector/builder
The vast majority of users will want to use selectors which are simple enough to represent as strings, such as
foo/bar
or*
. I think that, for the latter of "select all nodes", one could write today:This is my best guess - I don't think I understand the builder API, if I'm being honest.
It would be nice to, instead:
We already have
func ParseSelector(n ipld.Node) (Selector, error)
, which makes the naming perhaps a bit tricky.selector.ParseSelectorString
is possible, but also quite verbose as it repeats "selector". Ideally, we'd addParseString
, and we could deprecateParseSelector
in favor of a more accurate name likeParseNode
orReify
.There's also the question of how one would parse a string and obtain the node rather than the selector, but I don't think this will be common at all, so I don't think we need to worry about it. It seems like what one would want there, instead, is an inverse to
ParseSelector
to be able to see a selector as an IPLD node. But again, I don't have any use case for that either.cc @warpfork @willscott @ribasushi
The text was updated successfully, but these errors were encountered: