Skip to content
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

Consider empty strings in Text.indexOf #4101

Merged
merged 2 commits into from
Jun 16, 2023
Merged

Conversation

runarorama
Copy link
Contributor

Fixes this bug:

> ##Text.indexOf "" "foo"

⬇️

Encountered exception:
Data.Text.Lazy.breakOn: empty input
CallStack ( from HasCallStack ):
    error

The empty search string now just always returns the 0 index.

Copy link
Member

@pchiusano pchiusano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'd add a one line regression test to transcripts/builtins.md, @stew already has a bunch of examples there that you can follow.

test> Text.tests.indexOf =
   haystack = "01020304" ++ "05060708" ++ "090a0b0c01"
   needle1 = "01"
   needle2 = "02"
   needle3 = "0304"
   needle4 = "05"
   needle5 = "0405"
   needle6 = "0c"
   needle7 = haystack
   needle8 = "lopez"
   checks [
     Text.indexOf needle1 haystack == Some 0,
     Text.indexOf needle2 haystack == Some 2,
     Text.indexOf needle3 haystack == Some 4,
     Text.indexOf needle4 haystack == Some 8,
     Text.indexOf needle5 haystack == Some 6,
     Text.indexOf needle6 haystack == Some 22,
     Text.indexOf needle7 haystack == Some 0,
     Text.indexOf needle8 haystack == None,
   ]

@runarorama runarorama added the ready-to-merge Apply this to a PR and it will get merged automatically once CI passes and 1 reviewer has approved label Jun 15, 2023
@runarorama runarorama merged commit bbac67c into trunk Jun 16, 2023
@runarorama runarorama deleted the runarorama/textindexof branch June 16, 2023 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Apply this to a PR and it will get merged automatically once CI passes and 1 reviewer has approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants