superpower
superpower copied to clipboard
datalust
Visibility of Result<T>.Backtrack
I have tried to build an alternative tokenizer, based on the TokenizerBuilder in this repo. I hit a problem where the code for TokenizerBuilder uses Result<T>.Backtrack - this is an internal property. Which makes me wonder: if the current SimpleLinearTokenizer needs access to this property, wouldn't other custom tokenizers need it too?
To be more precise, this is the problematic part:
failure = new Result<TKind>()(remainder, augmentedMessage, attempt.Expectations, attempt.Backtrack);
It's not unlikely that custom tokenizer would need to augment the error message and position, just like the above statement.
..and this is what I needed in the end: https://github.com/ewoutkramer/superpower/commit/9c2674f28fec0dae575f117a5f6c6a0f856e6a3e
Thanks @ewoutkramer - those look like reasonable modifications, I'll take a look at this.
I have a different use case that needs Backtrack.
I want to write another combinator, similar to Many. This combinator would accept a lower and an upper bound, similar to regex {n,m}. I want my combinator to conform Many style but I cannot, since Many has access to Backtrack and I don't. @ewoutkramer solution unfortunately will not help that use case.
Looking at this again, I wonder if we should consider just making all the internal properties and constructors on Result<T> (and TokenListParserResult<T,U>) public 🤔
I originally steered away from this because I think they're pretty tightly bound to the internal implementation, but in retrospect, making experimentation and add-on development easier seems like a reasonable trade-off.
@nblumhardt As a user I support this, because it would allow me to do what I want. I'm not sure if I would be comfortable with it as a designer, but fortunately, I'm not the designer, you are. ;) So I'll leave it up to you.
As a fellow API designer we all know that you'll be stuck with your public interface for a long time. So, I'd do it just "on demand".
It probably does not make sense to open just a single property, if for nothing else, then for consistency reasons. They were designed as a part of an internal "interface" (I do not mean C# interface here, I mean it in more general sense) that we are considering making public here. If we are comfortable with this, it certainly makes sense. I cannot think of better options right now. I also agree, that it helps with experimentation and the longer term library enhancements.
👍 both angles are definitely valid. The current shape of these APIs has been stable for quite a long time now, so I'm leaning more towards favouring completeness and consistency in this case, and exposing all of the interface.