Merge pull request #26102 from JuliaLang/kf/shell_parse

Get rid of raw iteration protocol use in cmd parsing

Author: Keno

base/iterators.jl

@@ -1052,6 +1052,18 @@ mutable struct Stateful{T, VS}
     end
 end
 
+function reset!(s::Stateful{T,VS}, itr::T) where {T,VS}
+    s.itr = itr
+    state = start(itr)
+    if done(itr, state)
+        s.nextvalstate = nothing
+    else
+        s.nextvalstate = next(itr, state)::VS
+    end
+    s.taken = 0
+    s
+end
+
 # Try to find an appropriate type for the (value, state tuple),
 # by doing a recursive unrolling of the iteration protocol up to
 # fixpoint.

base/shell.jl

@@ -4,6 +4,18 @@
 
 const shell_special = "#{}()[]<>|&*?~;"
 
+# strips the end but respects the space when the string ends with "\\ "
+function rstrip_shell(s::AbstractString)
+    c_old = nothing
+    for (i, c) in Iterators.reverse(pairs(s))
+        ((c == '\\') && c_old == ' ') && return SubString(s, 1, i+1)
+        c in _default_delims || return SubString(s, 1, i)
+        c_old = c
+    end
+    SubString(s, 1, 0)
+end
+
+
 # needs to be factored out so depwarn only warns once
 # when removed, also need to update shell_escape for a Cmd to pass shell_special
 # and may want to use it in the test for #10120 (currently the implementation is essentially copied there)
@@ -12,24 +24,10 @@ const shell_special = "#{}()[]<>|&*?~;"
 
 function shell_parse(str::AbstractString, interpolate::Bool=true;
                      special::AbstractString="")
-    s = lstrip(str)
-    # strips the end but respects the space when the string ends with "\\ "
-    r = reverse(s)
-    i = start(r)
-    c_old = nothing
-    while !done(r,i)
-        c, j = next(r,i)
-        if c == '\\' && c_old == ' '
-            i -= 1
-            break
-        elseif !(c in _default_delims)
-            break
-        end
-        i = j
-        c_old = c
-    end
-    s = s[1:end-i+1]
+    s::SubString = SubString(str, firstindex(str))
+    s = rstrip_shell(lstrip(s))
 
+    # N.B.: This is used by REPLCompletions
     last_parse = 0:-1
     isempty(s) && return interpolate ? (Expr(:tuple,:()),last_parse) : ([],last_parse)
 
@@ -38,73 +36,69 @@ function shell_parse(str::AbstractString, interpolate::Bool=true;
 
     args::Vector{Any} = []
     arg::Vector{Any} = []
-    i = start(s)
-    j = i
+    i = firstindex(s)
+    st = Iterators.Stateful(pairs(s))
 
     function update_arg(x)
         if !isa(x,AbstractString) || !isempty(x)
             push!(arg, x)
         end
     end
+    function consume_upto(j)
+        update_arg(s[i:prevind(s, j)])
+        i = coalesce(peek(st), (lastindex(s)+1,'\0'))[1]
+    end
     function append_arg()
         if isempty(arg); arg = Any["",]; end
         push!(args, arg)
         arg = []
     end
 
-    while !done(s,j)
-        c, k = next(s,j)
+    for (j, c) in st
         if !in_single_quotes && !in_double_quotes && isspace(c)
-            update_arg(s[i:prevind(s, j)])
+            consume_upto(j)
             append_arg()
-            j = k
-            while !done(s,j)
-                c, k = next(s,j)
-                if !isspace(c)
-                    i = j
-                    break
-                end
-                j = k
+            while !isempty(st)
+                # We've made sure above that we don't end in whitespace,
+                # so updateing `i` here is ok
+                (i, c) = peek(st)
+                isspace(c) || break
+                popfirst!(st)
             end
         elseif interpolate && !in_single_quotes && c == '$'
-            update_arg(s[i:prevind(s, j)]); i = k; j = k
-            if done(s,k)
-                error("\$ right before end of command")
-            end
-            if isspace(s[k])
-                error("space not allowed right after \$")
-            end
-            stpos = j
-            ex, j = Meta.parse(s,j,greedy=false)
-            last_parse = stpos:j
-            update_arg(ex); i = j
+            consume_upto(j)
+            isempty(st) && error("\$ right before end of command")
+            stpos, c = popfirst!(st)
+            isspace(c) && error("space not allowed right after \$")
+            ex, j = Meta.parse(s,stpos,greedy=false)
+            last_parse = (stpos:prevind(s, j)) .+ s.offset
+            update_arg(ex);
+            s = SubString(s, j)
+            Iterators.reset!(st, pairs(s))
+            i = firstindex(s)
         else
             if !in_double_quotes && c == '\''
                 in_single_quotes = !in_single_quotes
-                update_arg(s[i:prevind(s, j)]); i = k
+                consume_upto(j)
             elseif !in_single_quotes && c == '"'
                 in_double_quotes = !in_double_quotes
-                update_arg(s[i:prevind(s, j)]); i = k
+                consume_upto(j)
             elseif c == '\\'
                 if in_double_quotes
-                    if done(s,k)
-                        error("unterminated double quote")
-                    end
-                    if s[k] == '"' || s[k] == '$' || s[k] == '\\'
-                        update_arg(s[i:prevind(s, j)]); i = k
-                        c, k = next(s,k)
+                    isempty(st) && error("unterminated double quote")
+                    k, c′ = peek(st)
+                    if c′ == '"' || c′ == '$' || c′ == '\\'
+                        consume_upto(j)
+                        _ = popfirst!(st)
                     end
                 elseif !in_single_quotes
-                    if done(s,k)
-                        error("dangling backslash")
-                    end
-                    update_arg(s[i:prevind(s, j)]); i = k
-                    c, k = next(s,k)
+                    isempty(st) && error("dangling backslash")
+                    consume_upto(j)
+                    _ = popfirst!(st)
                 end
             elseif !in_single_quotes && !in_double_quotes && c in special
                 warn_shell_special(special) # noinline depwarn
             end
-            j = k
         end
     end
 

base/show.jl

@@ -766,15 +766,10 @@ const expr_parens = Dict(:tuple=>('(',')'), :vcat=>('[',']'),
 is_id_start_char(c::Char) = ccall(:jl_id_start_char, Cint, (UInt32,), c) != 0
 is_id_char(c::Char) = ccall(:jl_id_char, Cint, (UInt32,), c) != 0
 function isidentifier(s::AbstractString)
-    i = start(s)
-    done(s, i) && return false
-    (c, i) = next(s, i)
+    isempty(s) && return false
+    c, rest = Iterators.peel(s)
     is_id_start_char(c) || return false
-    while !done(s, i)
-        (c, i) = next(s, i)
-        is_id_char(c) || return false
-    end
-    return true
+    return all(is_id_char, rest)
 end
 isidentifier(s::Symbol) = isidentifier(string(s))
 

stdlib/REPL/src/REPLCompletions.jl

@@ -646,10 +646,9 @@ function shell_completions(string, pos)
 
         return complete_path(prefix, pos, use_envpath=use_envpath)
     elseif isexpr(arg, :incomplete) || isexpr(arg, :error)
-        r = first(last_parse):prevind(last_parse, last(last_parse))
-        partial = scs[r]
+        partial = scs[last_parse]
         ret, range = completions(partial, lastindex(partial))
-        range = range .+ (first(r) - 1)
+        range = range .+ (first(last_parse) - 1)
         return ret, range, true
     end
     return String[], 0:-1, false

test/spawn.jl

@@ -527,3 +527,8 @@ let p = spawn(`$sleepcmd 100`)
     # Should not throw if already dead
     kill(p)
 end
+
+# Second argument of shell_parse
+let s = "   \$abc   "
+    @test s[Base.shell_parse(s)[2]] == "abc"
+end