Add method to contains for regex needle

[dhoegh]

This PR defines contains with a regex search pattern. Besides the new method I have also widen the signature of the original method to include Char as: contains(haystack::AbstractString, needle::Union{AbstractString, Char}). Besides the additions to contains, I have:

• Moved the docs for the functions in search inline
• Moved the test for search(::String, ::Regex) from test/strings/search to test/regex.jl as the method is defined in regex.jl.
• Added methods to contains and test for the methods plus additional tests for contains in general as it seemed not to be tested directly anywhere.
• Fixed an reference to ASCII and UTF8 string.

It would be good to backport this to 0.5.1 as it increases test coverage and fixes docs. The additional methods for contains are probably not to be viewed as a "new" feature?

[ararslan]

The additional methods for contains are probably not to be viewed as a "new" feature?

It seems to me like it would constitute a new feature, though that's of course up to one of the owners to decide.

This PR defines contains with a regex search pattern.

We already have this functionality with the ismatch function, so IMHO it would be kind of confusing to have it duplicated in contains, especially because the order of the arguments is reversed. (ismatch takes the regex first, your contains method takes the regex second.)

The method with Char seems okay to me, though for that we also have in.

Anyway, that's my $0.02.

[dhoegh]

@ararslan you are correct, the functionality are there today. But from my point of view the current state is cluttered, as a test if a string contains a substring/regex/char requires different functions, see example:

#For characters use `in`
'H' in "Hello"
# For strings 
contains("Hello", "He")
# For regex pattern
ismatch(r"He","Hello")

From my point of view, Julia should unify this interface better using multiple dispatch. I would propose these three tests should use the same function and then deprecate all others. Then this is definitely first in 0.6. I do not care for the name or argument order. But i think the name should reflect the order. in suggest the needle argument first, contains suggest needle last, ismatch do to me not reflect a certain order of the arguments.

in(::String, ::String) has implemented an error that states you should use contians. The error message was implemented in 143395e. The error was probly implemented due to a decision. cc @JeffBezanson.

From my perspective the best choice seem to unify on using in as:

#For characters use `in`
'H' in "Hello"
# For strings 
"He" in "Hello"
# For regex pattern
r"He" in "Hello"

Python do also use in for characters and strings, but not for regex.

[ararslan]

Hm, I'm not so much a fan of using in that way. In a broader sense, in typically means "element in collection," and strings are essentially collections of Chars. So IMO "regex in collection" or even "string in collection" seems a little iffy based on that definition, as neither regular expressions nor strings are elements of strings.

With this in mind, I think the current separation of in, contains, and ismatch makes sense, as they all mean slightly different things.

• a in b: Is a an element in the collection b?
• contains(b, a): Does the string b contain exactly the string a?
• ismatch(r, b): Does the string b have at least one match of the regular expression r?

And at least IMO, ismatch shouldn't be deprecated as it belongs in the match and matchall family.

But as I say often around these parts, my opinion here doesn't really matter since I'm just some guy, I just figured I'd state my position on it anyway, just in case. 😄

[kmsquire]

But as I say often around these parts, my opinion here doesn't really matter since I'm just some guy, I just figured I'd state my position on it anyway, just in case. 😄

@ararslan, most of us are just "some guy/gal", and your opinion certainly counts, especially with your recent involvement. (IMHO, only uninformed and rudely expressed opinions count less, so I think you're safe...)

I actually agree that, in general, a streamlined interface makes sense. However, my preference is probably to get rid of contains all together.

If we decide to keep it, though, I think this is a good change.

[StefanKarpinski]

I could see merging ismatch into contains since there's no other sensible way in which a string could contain a regex pattern, but as @ararslan points out, in is quite different and test for element containment.

[dhoegh]

Sorry hit the wrong button.

I see a regex as a way of specifying what you look for. As an example a regex could explain to look for a needle with a size of 4-5cm in the haystack. With this in mind I think it makes sense to use contains.

[stevengj]

Why not just call ismatch(re, haystack)?

[simonbyrne]

I hadn't seen this PR when I opened #19250. Is there any chance you could move the docs changes to a separate PR (which should be straightforward to merge), then we can hash out the changes here?

[JeffBezanson]

Bump. I think we should do two things here. (1) Merge ismatch and contains. (2) contains has another method (contains(eq::Function, itr, x)) that's unrelated to substrings and has a weird signature; it should be contains(predicate, itr). The signature should be changed, and the function possibly renamed.

[JeffBezanson]

From triage: ismatch sounds like it requires the entire string to match the regex, when in fact it tests whether the string contains a match of the regex. So absorbing ismatch into contains seems like a good idea to me. (Plus as @simonbyrne points out in #19250, search and replace already accept both strings and regexes interchangeably.)

[StefanKarpinski]

Are we planning on replacing match as well? Python does actually use re.match(str) to mean that the regular expression matches the entire string rather than a substring of it.

[stevengj]

If it were up to me, I'd be tempted to rename match to findfirst, since we already have the latter and "findfirst" is more descriptive of what it actually does.

[ararslan]

findfirst returns an index though, whereas match returns a RegexMatch. I don't think findfirst would be an appropriate replacement.

[nalimilan]

@dhoegh Given what seems to be the consensus above, could you rebase the PR and keep only the changes deprecating ismatch in favor of contains? Docstring changes would better go to another PR (and anyway I'm working on deprecating search altogether).

[nalimilan]

I've added a commit merging ismatch into contains to PR #24673. Closing.