fixed between function #946
Conversation
There was a problem hiding this comment.
Hey there @mohammadKhalafi,
Thank you so much for your interest in Mavo and your PR. It indeed fixes the bug, and all the tests we have still pass. That's awesome!
However, I found a use case where the between function doesn't work as we expect (see the comment). Is there a chance you could fix that issue as well?
| if (tight && toIndex <= fromIndex){ | ||
| return haystack.slice(toIndex + to.length, fromIndex); | ||
| } | ||
|
|
||
| return haystack.slice(fromIndex + from.length, toIndex); |
There was a problem hiding this comment.
But what if from and/or to are not provided (either of them can be an empty string)? They are optional by design (and you took it into account in your first commit). Moreover, the fromlast() and tofirst() functions are built on top of the optionality of from and to.
E.g., With this change, fromlast("foo.bar.baz", ".") returns foo.bar instead of baz, and tofirst("foo.bar.baz", ".") returns bar.baz instead of foo.
|
@LeaVerou Lea, I reviewed this PR. And it looks like now everything works as it should: the tests pass, the bug is fixed, I also checked our demos—no regression. Do you mind me merging this PR? |
If I assign you a review, it means I trust you to merge when you think it's ready 😄 |
Got it. Thanks! 😄 |
|
@mohammadKhalafi Thank you so much for your work! |
issue link : #915