Add common Functor#as directly #52
Conversation
| /** Replaces parsed values with the given value. | ||
| */ | ||
| def as[B](b: B): Parser[B] = | ||
| Parser.as(this)(b) |
There was a problem hiding this comment.
I think here we can just do .void.map(_ => b) since this is not fundamental. And remove from the companion.
| /** This method override `Parser#replicateA` to refine return type. | ||
| */ | ||
| override def replicateA(n: Int): Parser1[List[A]] = | ||
| Parser.replicateA1(n, this) |
There was a problem hiding this comment.
This isn't a Parser1 if n == 0, right?
| /** This method overrides `Parser#as` to refine the return type. | ||
| */ | ||
| override def as[B](b: B): Parser1[B] = | ||
| Parser.as1(this)(b) |
There was a problem hiding this comment.
I think void.map is fine.
| /** Replaces parsed values with the given value. | ||
| */ | ||
| def as[A, B](pa: Parser[A])(b: B): Parser[B] = | ||
| map(Impl.Void(pa))(_ => b) |
There was a problem hiding this comment.
Actually def void does more than this. It also unmaps pa so we can simplify it. We should use that but I think as I mentioned just using the void method is the way to go.
There was a problem hiding this comment.
Yeah you are right, it's better to use it.
| } | ||
| } | ||
|
|
||
| case class Replicate1[A, B](p1: Parser1[A], n: Int, acc: Accumulator1[A, B]) |
There was a problem hiding this comment.
What do you think of this idea instead: we take a min and a max in Rep, and for replicate then min is the max. But this also enables a new combinator: parse at most n things. Of course that is a bit of a weird combinator so maybe not with special casing.
| /** Apply Parser1 `n` times | ||
| */ | ||
| def replicateA1[A](n: Int, pa: Parser1[A])(implicit | ||
| acc: Accumulator1[A, List[A]] |
There was a problem hiding this comment.
I think we should just pass the List accumulator and not use an implicit here since it compilcates the call site but does not add power. What do you think?
Alternatively: we make this replicateAs and then allow us to replicate into different types (like Vector)
|
Thanks for taking this on! |
|
The more I think about it, replicateA is a really bad name. The A comes from the fact that replicate is already a method on collections, IIRC. So, the A stands for applicative. I think maybe .exactly or something might be a better name. I used rep, following fastparse, to mean repeat at least, which is the most common kind of repetition (and almost always you want to repeat 0 or more times, or 1 or more times)... honestly, maybe we should just punt on the whole replicate question and remove that for now. I can't even imagine a use case at the moment, and if you want it, you can use replicateA from Applicative to get it. What do you think? |
|
@johnynek I agree with you regarding I'll clean it up in a bit! |
regadas
changed the title
|
Thanks a ton! I'm going to make one more PR than release a new version today. |
No description provided.