-
-
Notifications
You must be signed in to change notification settings - Fork 157
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 not_ref() #570
base: main
Are you sure you want to change the base?
Add not_ref() #570
Conversation
Hmm, I worry that we're going to end up duplicating a lot of combinators to make this work across the crate. I think there probably needs to be a better way of generating errors without relying on being able to pull values out by-value. I need to revisit exactly how error generation works with |
Yeah I do agree, relaxing the trait bound from |
@zesterer I'm thinking I'll have to publish my own variant of this crate as an interim measure, so I can publish (an update to) my beancount-parser-lima crate which depends on it. Best practice in the community seems to be I publish a crate called tesujimath-chumsky with a clear description as to it's temporary nature, so publishing my own crate isn't blocked this. As soon as there's a published solution to what was blocking me here and in #569, I'll delete my crate and switch back. Ok? |
Apologies, I've had a really busy few weeks (new job, among other things!). This is on my todo list (along with a series of other things), I'll try to get a decent response up here by the end of the weekend. |
Great, and thank you! Sorry if it seems like I'm hassling you about this, that's not my intention. Really appreciate your work on Chumsky! |
@zesterer I had another look at what would be required to support Relaxing the trait constraints from For now I have only two ideas:
Since my tokens come from Logos and contain |
One existing issue with
This seems... counterintuitive, to say the least given that an The wider issue here is that we don't have a good way to fall back on a non-token pattern. Ideally, the error would be more like If this wider problem were solved, then I suspect that the It's a tricky one: while I definitely see the value in this combinator, I'm also keenly aware that the reason for its existence is a product of poor design elsewhere in the crate. I think I'd be happy to accept it, but perhaps only as a temporary measure: perhaps it could be put behind the
The difficulty here is that the inputs supported by chumsky have mutually-exclusive feature sets.
The only commonality between these inputs is that they must be able to produce a |
I wasn't able to transition to But by restructuring my parser, I was able to avoid needing both Notwithstanding the Thanks for continuing to make Chumsky more awesome! ❤️ |
Good to hear that you've been able to work around it. I'm going to keep this PR open for now, I if only as a reminder to myself to come up with a good solution to this wider problem. |
Works just like
not()
but forBorrowInput
rather thanValueInput
.This is just like I did for
any_ref
, and needed for the same reason, that is, becausenot
can't be used forBorrowInput
, onlyValueInput