-
Notifications
You must be signed in to change notification settings - Fork 8
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
Implement toBe(LITERAL)
, not just toBe_TODO()
#49
Conversation
So this test passes selfie/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt Lines 34 to 40 in c04b9b4
And this one fails selfie/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt Lines 42 to 49 in c04b9b4
Because
@jknack can you take a whack at finishing this PR sometime this week if you have time? Once you do, we should be able to run through the rest of the roadmap without much obstacle: |
I just fixed up this branch with all the changes on main, but the tasks above are unchanged. |
Sorry @jknack, I deleted this branch on accident, it's still a needed WIP and the next piece of work I have for you if you end up having any more time in 2023. |
@nedtwigg implemented handling of () while parsing .toBe |
- parse '()' to extract literal values - TODO: do we need to handle nested '()' - TODO: handle '()' inside string literal
3682c46
to
92795b4
Compare
Signed-off-by: Ned Twigg <[email protected]>
var opened = 0 | ||
val startIndex = idx + prefix.length | ||
var endIndex = -1 | ||
// TODO: do we need to detect paired parenthesis ( ( ) )? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cases we need to handle:
.toBe("""
(((
"
(((
""")
// or
.toBe(
"(((\n" +
"\"\n" +
"(((")
Cases we do not need to handle
.toBe(("a" + "b") + "c")
We should add support for these in future PRs, there are many hard cases, we should do the easy stuff first ("""
literals) and then solve problems as they come.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huzzah!!
Happy holidays @jknack, great to have you back! I will merge this, it's great! Next up you can burn through this list: For testing on many of them, I think we don't need the overhead of the |
No description provided.