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

Rename ismatch to occursin; fix 0.7 depwarns #15

Merged
merged 3 commits into from
Jun 20, 2018
Merged

Conversation

jmkuhn
Copy link
Contributor

@jmkuhn jmkuhn commented Jun 19, 2018

ismatch has been renamed to occursin in Julia Base.
JuliaLang/julia#24673
JuliaLang/julia#26283

To accommodate this in Glob, for Julia 0.6 allow
either ismatch or occursin to be used.

julia> using Glob

julia> using Compat

julia> ismatch(fn"AB*AB*AB", "ABXABXAB")
true

julia> occursin(fn"AB*AB*AB", "ABXABXAB")
true

With the current 0.7 nightly, ismatch produces a deprecation warning.

julia> using Glob

julia> ismatch(fn"AB*AB*AB", "ABXABXAB")
┌ Warning: `ismatch(fn::FilenameMatch, s::AbstractString)` is deprecated, use `occursin(fn, s)` instead.
│   caller = top-level scope
└ @ Core :0
true

julia> occursin(fn"AB*AB*AB", "ABXABXAB")
true

I'm not sure my updates to the iterator are the best way to do this. If you have any suggestions, let me know.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.1%) to 91.866% when pulling d1a4a75 on jmkuhn:0.7 into 8ca3670 on vtjnash:master.

@coveralls
Copy link

coveralls commented Jun 19, 2018

Coverage Status

Coverage increased (+2.3%) to 93.056% when pulling 442c15e on jmkuhn:0.7 into 8ca3670 on vtjnash:master.

Copy link
Owner

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

Thanks! It's looking good. I've left some comments on the best way to update these.

src/Glob.jl Outdated
match = false # string characters left to match, but no pattern left
else
mc, mi = next(pattern, mi)
mc, mi = iterate(pattern, mi)
Copy link
Owner

Choose a reason for hiding this comment

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

Use the following pattern here:

     patnext = iterate(pattern, mi)
     if patnext === nothing
        match = false # string characters left to match, but no pattern left
     else
          mc, mi = patnext

src/Glob.jl Outdated
while !done(pattern, mi) # allow trailing *'s
mc, mi = next(pattern, mi)
while mi <= ncodeunits(pattern) # allow trailing *'s
mc, mi = iterate(pattern, mi)
Copy link
Owner

Choose a reason for hiding this comment

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

Rewrite this as:

     while true # allow trailing *'s
         patnext = iterate(pattern, mi)
         patnext === nothing && break
         mc, mi = patnext
         mc == '*' || return false # pattern characters left to match, but no string left
     end

src/Glob.jl Outdated
if period & (c == '.')
return false # * does not match leading .
end
match = true
else
c, i = next(s, i)
c, i = iterate(s, i)
Copy link
Owner

Choose a reason for hiding this comment

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

Can we update this method to compute this at the top:

i = firstindex(s)
while true
    matchnext = iterate(s, i)
    matchnext === nothing && break
...
    c, _ = matchnext # peek-ahead
...
    c, i = matchnext
...
end

src/Glob.jl Outdated
if (!noescape) & (mc == '\\') & (!done(pattern, mi))
mc, mi = next(pattern, mi)
if (!noescape) & (mc == '\\') & (mi <= ncodeunits(pattern))
mc, mi = iterate(pattern, mi)
end
Copy link
Owner

Choose a reason for hiding this comment

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

    if (!noescape) & (mc == '\\') # escape the next character after backslash, unless it is the last character
        patnext = iterate(pattern, mi)
        if patnext !== nothing
            mc, mi = patnext
        end
    end

src/Glob.jl Outdated
end
match = ((c == mc) || (caseless && uppercase(c)==uppercase(mc)))
end
end
end
if !match # try to backtrack and add another character to the last *
star == 0 && return false
c, i = next(s, starmatch)
c, i = iterate(s, starmatch)
Copy link
Owner

Choose a reason for hiding this comment

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

    c, i = something(iterate(s, starmatch)) # starmatch is strictly <= i, so it is known that it must be a valid index

src/Glob.jl Outdated
@@ -179,13 +201,13 @@ function _match_bracket(pat::AbstractString, mc::Char, i, cl::Char, cu::Char) #
#match = (pat[j:k0] == s[ci:ci+(k0-j)])
#return (mc, k3, true, match)
end
mc, j = next(pat, j)
mc, j = iterate(pat, j)
Copy link
Owner

Choose a reason for hiding this comment

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

wrap in something() (like above)

src/Glob.jl Outdated
return (mc, k3, false, true)
else #if mc2 == '='
if j != k0
error(string("only single characters are currently supported as character equivalents, got [=", SubString(pat, j, k0), "=]"))
end
mc, j = next(pat, j)
mc, j = iterate(pat, j)
Copy link
Owner

Choose a reason for hiding this comment

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

wrap in something() (like above)

src/Glob.jl Outdated
return (i0, false, c=='[')
end
mc, j = next(pat, i)
mc, j = iterate(pat, i)
Copy link
Owner

Choose a reason for hiding this comment

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

like you did above, use:

next = iterate(pat, i)
if next === nothing
    return (i0, false, c=='[')
end
mc, j = next

src/Glob.jl Outdated
while !done(pat,i)
mc, i = next(pat, i)
while i <= ncodeunits(pat)
mc, i = iterate(pat, i)
Copy link
Owner

Choose a reason for hiding this comment

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

similar to above, use:

while true
    next = iterate(pat, i)
    next === nothing && break
    mc, i = next
...
end

src/Glob.jl Outdated
@@ -226,20 +248,20 @@ function _match(pat::AbstractString, i0, c::Char, caseless::Bool, extended::Bool
return (i0, false, c=='[')
end
elseif extended & (mc == '\\')
if done(pat, i)
if i > ncodeunits(pat)
Copy link
Owner

Choose a reason for hiding this comment

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

similarly for all of these lines, use the assignment to the temporary next variable pattern for these that follow here

@jmkuhn
Copy link
Contributor Author

jmkuhn commented Jun 20, 2018

Thanks for the thorough review. The iterator code looks much better now.

@vtjnash vtjnash merged commit 52e0ed0 into vtjnash:master Jun 20, 2018
@jmkuhn jmkuhn deleted the 0.7 branch June 20, 2018 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants