Rename split keyword from keep to keepempty (#26647)

This is a much clearer name. I've also enabled various forms of split/rsplit to be consistent with the docs.

Author: simonbyrne

base/deprecated.jl

@@ -1570,6 +1570,11 @@ function search(buf::IOBuffer, delim::UInt8)
     return Int(q-p+1)
 end
 
+# PR #26647
+# The `keep` argument in `split` and `rpslit` has been renamed to `keepempty`.
+# To remove this deprecation, remove the `keep` argument from the function signatures as well as
+# the internal logic that deals with the renaming. These live in base/strings/util.jl.
+
 # END 0.7 deprecations
 
 # BEGIN 1.0 deprecations

base/strings/util.jl

@@ -239,8 +239,8 @@ function rpad(
 end
 
 """
-    split(s::AbstractString; limit::Integer=0, keep::Bool=false)
-    split(s::AbstractString, chars; limit::Integer=0, keep::Bool=true)
+    split(s::AbstractString; limit::Integer=0, keepempty::Bool=false)
+    split(s::AbstractString, chars; limit::Integer=0, keepempty::Bool=true)
 
 Return an array of substrings by splitting the given string on occurrences of the given
 character delimiters, which may be specified in any of the formats allowed by
@@ -251,9 +251,11 @@ If `chars` is omitted, it defaults to the set of all space characters.
 
 The optional keyword arguments are:
  - `limit`: the maximum size of the result. `limit=0` implies no maximum (default)
- - `keep`: whether empty fields should be kept in the result. Default is `false` without
+ - `keepempty`: whether empty fields should be kept in the result. Default is `false` without
    a `chars` argument, `true` with a `chars` argument.
 
+See also [`rsplit`](@ref).
+
 # Examples
 ```jldoctest
 julia> a = "Ma.rch"
@@ -267,25 +269,40 @@ julia> split(a,".")
 """
 function split end
 
-split(str::T, splitter;
-      limit::Integer=0, keep::Bool=true) where {T<:AbstractString} =
-    _split(str, splitter, limit, keep, T <: SubString ? T[] : SubString{T}[])
-split(str::T, splitter::Union{Tuple{Vararg{<:AbstractChar}},AbstractVector{<:AbstractChar},Set{<:AbstractChar}};
-      limit::Integer=0, keep::Bool=true) where {T<:AbstractString} =
-    _split(str, in(splitter), limit, keep, T <: SubString ? T[] : SubString{T}[])
-split(str::T, splitter::AbstractChar;
-      limit::Integer=0, keep::Bool=true) where {T<:AbstractString} =
-    _split(str, isequal(splitter), limit, keep, T <: SubString ? T[] : SubString{T}[])
-
-function _split(str::AbstractString, splitter, limit::Integer, keep_empty::Bool, strs::Array)
+function split(str::T, splitter;
+               limit::Integer=0, keepempty::Bool=true, keep::Union{Nothing,Bool}=nothing) where {T<:AbstractString}
+    if keep !== nothing
+        Base.depwarn("The `keep` keyword argument is deprecated; use `keepempty` instead", :split)
+        keepempty = keep
+    end
+    _split(str, splitter, limit, keepempty, T <: SubString ? T[] : SubString{T}[])
+end
+function split(str::T, splitter::Union{Tuple{Vararg{<:AbstractChar}},AbstractVector{<:AbstractChar},Set{<:AbstractChar}};
+               limit::Integer=0, keepempty::Bool=true, keep::Union{Nothing,Bool}=nothing) where {T<:AbstractString}
+    if keep !== nothing
+        Base.depwarn("The `keep` keyword argument is deprecated; use `keepempty` instead", :split)
+        keepempty = keep
+    end
+    _split(str, in(splitter), limit, keepempty, T <: SubString ? T[] : SubString{T}[])
+end
+function split(str::T, splitter::AbstractChar;
+               limit::Integer=0, keepempty::Bool=true, keep::Union{Nothing,Bool}=nothing) where {T<:AbstractString}
+    if keep !== nothing
+        Base.depwarn("The `keep` keyword argument is deprecated; use `keepempty` instead", :split)
+        keepempty = keep
+    end
+    _split(str, isequal(splitter), limit, keepempty, T <: SubString ? T[] : SubString{T}[])
+end
+
+function _split(str::AbstractString, splitter, limit::Integer, keepempty::Bool, strs::Array)
     i = 1 # firstindex(str)
     n = lastindex(str)
     r = coalesce(findfirst(splitter,str), 0)
     if r != 0:-1
         j, k = first(r), nextind(str,last(r))
         while 0 < j <= n && length(strs) != limit-1
             if i < k
-                if keep_empty || i < j
+                if keepempty || i < j
                     push!(strs, SubString(str,i,prevind(str,j)))
                 end
                 i = k
@@ -296,17 +313,20 @@ function _split(str::AbstractString, splitter, limit::Integer, keep_empty::Bool,
             j, k = first(r), nextind(str,last(r))
         end
     end
-    if keep_empty || !done(str,i)
+    if keepempty || !done(str,i)
         push!(strs, SubString(str,i))
     end
     return strs
 end
 
 # a bit oddball, but standard behavior in Perl, Ruby & Python:
-split(str::AbstractString) = split(str, _default_delims; limit=0, keep=false)
+split(str::AbstractString;
+      limit::Integer=0, keepempty::Bool=false) =
+    split(str, _default_delims; limit=limit, keepempty=keepempty)
 
 """
-    rsplit(s::AbstractString, [chars]; limit::Integer=0, keep::Bool=true)
+    rsplit(s::AbstractString; limit::Integer=0, keepempty::Bool=false)
+    rsplit(s::AbstractString, chars; limit::Integer=0, keepempty::Bool=true)
 
 Similar to [`split`](@ref), but starting from the end of the string.
 
@@ -335,29 +355,47 @@ julia> rsplit(a,".";limit=2)
 """
 function rsplit end
 
-rsplit(str::T, splitter; limit::Integer=0, keep::Bool=true) where {T<:AbstractString} =
-    _rsplit(str, splitter, limit, keep, T <: SubString ? T[] : SubString{T}[])
-rsplit(str::T, splitter::Union{Tuple{Vararg{<:AbstractChar}},AbstractVector{<:AbstractChar},Set{<:AbstractChar}};
-       limit::Integer=0, keep::Bool=true) where {T<:AbstractString} =
-  _rsplit(str, in(splitter), limit, keep, T <: SubString ? T[] : SubString{T}[])
-rsplit(str::T, splitter::AbstractChar;
-       limit::Integer=0, keep::Bool=true) where {T<:AbstractString} =
-  _rsplit(str, isequal(splitter), limit, keep, T <: SubString ? T[] : SubString{T}[])
+function rsplit(str::T, splitter;
+                limit::Integer=0, keepempty::Bool=true, keep::Union{Nothing,Bool}=nothing) where {T<:AbstractString}
+    if keep !== nothing
+        Base.depwarn("The `keep` keyword argument is deprecated; use `keepempty` instead", :rsplit)
+        keepempty = keep
+    end
+    _rsplit(str, splitter, limit, keepempty, T <: SubString ? T[] : SubString{T}[])
+end
+function rsplit(str::T, splitter::Union{Tuple{Vararg{<:AbstractChar}},AbstractVector{<:AbstractChar},Set{<:AbstractChar}};
+                limit::Integer=0, keepempty::Bool=true, keep::Union{Nothing,Bool}=nothing) where {T<:AbstractString}
+    if keep !== nothing
+        Base.depwarn("The `keep` keyword argument is deprecated; use `keepempty` instead", :rsplit)
+        keepempty = keep
+    end
+    _rsplit(str, in(splitter), limit, keepempty, T <: SubString ? T[] : SubString{T}[])
+end
+function rsplit(str::T, splitter::AbstractChar;
+                limit::Integer=0, keepempty::Bool=true, keep::Union{Nothing,Bool}=nothing) where {T<:AbstractString}
+    if keep !== nothing
+        Base.depwarn("The `keep` keyword argument is deprecated; use `keepempty` instead", :rsplit)
+        keepempty = keep
+    end
+    _rsplit(str, isequal(splitter), limit, keepempty, T <: SubString ? T[] : SubString{T}[])
+end
 
-function _rsplit(str::AbstractString, splitter, limit::Integer, keep_empty::Bool, strs::Array)
+function _rsplit(str::AbstractString, splitter, limit::Integer, keepempty::Bool, strs::Array)
     n = lastindex(str)
     r = coalesce(findlast(splitter, str), 0)
     j, k = first(r), last(r)
     while j > 0 && k > 0 && length(strs) != limit-1
-        (keep_empty || k < n) && pushfirst!(strs, SubString(str,nextind(str,k),n))
+        (keepempty || k < n) && pushfirst!(strs, SubString(str,nextind(str,k),n))
         n = prevind(str, j)
         r = coalesce(findprev(splitter,str,n), 0)
         j, k = first(r), last(r)
     end
-    (keep_empty || n > 0) && pushfirst!(strs, SubString(str,1,n))
+    (keepempty || n > 0) && pushfirst!(strs, SubString(str,1,n))
     return strs
 end
-#rsplit(str::AbstractString) = rsplit(str, _default_delims, 0, false)
+rsplit(str::AbstractString;
+      limit::Integer=0, keepempty::Bool=false) =
+    rsplit(str, _default_delims; limit=limit, keepempty=keepempty)
 
 _replace(io, repl, str, r, pattern) = print(io, repl)
 _replace(io, repl::Function, str, r, pattern) =

stdlib/Distributed/src/cluster.jl

@@ -1153,8 +1153,8 @@ end
 
 function load_machine_file(path::AbstractString)
     machines = []
-    for line in split(read(path, String),'\n'; keep=false)
-        s = split(line, '*'; keep = false)
+    for line in split(read(path, String),'\n'; keepempty=false)
+        s = split(line, '*'; keepempty=false)
         map!(strip, s, s)
         if length(s) > 1
             cnt = all(isdigit, s[1]) ? parse(Int,s[1]) : Symbol(s[1])

stdlib/Pkg3/src/REPLMode.jl

@@ -802,7 +802,7 @@ end
 function completions(full, index)
     pre = full[1:index]
 
-    pre_words = split(pre, ' ', keep=true)
+    pre_words = split(pre, ' ', keepempty=true)
 
     # first word should always be a command
     if isempty(pre_words)

stdlib/REPL/docs/src/index.md

@@ -290,13 +290,13 @@ max(x, y) in Base at operators.jl:215
 max(a, b, c, xs...) in Base at operators.jl:281
 ```
 
-Keywords are also displayed in the suggested methods, see second line after `;` where `limit`
-and `keep` are keyword arguments:
+Keywords are also displayed in the suggested methods after `;`, see below line where `limit`
+and `keepempty` are keyword arguments:
 
 ```julia-repl
 julia> split("1 1 1", [TAB]
-split(str::AbstractString) in Base at strings/util.jl:302
-split(str::T, splitter; limit, keep) where T<:AbstractString in Base at strings/util.jl:277
+split(str::AbstractString; limit, keepempty) in Base at strings/util.jl:302
+split(str::T, splitter; limit, keepempty) where T<:AbstractString in Base at strings/util.jl:277
 ```
 
 The completion of the methods uses type inference and can therefore see if the arguments match

test/strings/util.jl

@@ -88,13 +88,18 @@ end
     @test split("", ',') == [""]
     @test split(",", ',') == ["",""]
     @test split(",,", ',') == ["","",""]
-    @test split("", ','  ; keep=false) == []
-    @test split(",", ',' ; keep=false) == []
-    @test split(",,", ','; keep=false) == []
+    @test split("", ','  ; keepempty=false) == []
+    @test split(",", ',' ; keepempty=false) == []
+    @test split(",,", ','; keepempty=false) == []
 
     @test split("a b c") == ["a","b","c"]
     @test split("a  b \t c\n") == ["a","b","c"]
 
+    @test split("a b c"; limit=2) == ["a","b c"]
+    @test split("a  b \t c\n"; limit=3) == ["a","b","\t c\n"]
+    @test split("a b c"; keepempty=true) == ["a","b","c"]
+    @test split("a  b \t c\n"; keepempty=true) == ["a","","b","","","c",""]
+
     @test rsplit("foo,bar,baz", 'x') == ["foo,bar,baz"]
     @test rsplit("foo,bar,baz", ',') == ["foo","bar","baz"]
     @test rsplit("foo,bar,baz", ",") == ["foo","bar","baz"]
@@ -108,24 +113,29 @@ end
     @test rsplit(",", ',') == ["",""]
     @test rsplit(",,", ',') == ["","",""]
     @test rsplit(",,", ','; limit=2) == [",",""]
-    @test rsplit("", ','  ; keep=false) == []
-    @test rsplit(",", ',' ; keep=false) == []
-    @test rsplit(",,", ','; keep=false) == []
+    @test rsplit("", ','  ; keepempty=false) == []
+    @test rsplit(",", ',' ; keepempty=false) == []
+    @test rsplit(",,", ','; keepempty=false) == []
+
+    @test rsplit("a b c") == ["a","b","c"]
+    @test rsplit("a  b \t c\n") == ["a","b","c"]
 
-    #@test rsplit("a b c") == ["a","b","c"]
-    #@test rsplit("a  b \t c\n") == ["a","b","c"]
+    @test rsplit("a b c"; limit=2) == ["a b", "c"]
+    @test rsplit("a  b \t c\n"; limit=3) == ["a ","b","c"]
+    @test rsplit("a b c"; keepempty=true) == ["a","b","c"]
+    @test rsplit("a  b \t c\n"; keepempty=true) == ["a","","b","","","c",""]
 
     let str = "a.:.ba..:..cba.:.:.dcba.:."
     @test split(str, ".:.") == ["a","ba.",".cba",":.dcba",""]
-    @test split(str, ".:."; keep=false) == ["a","ba.",".cba",":.dcba"]
+    @test split(str, ".:."; keepempty=false) == ["a","ba.",".cba",":.dcba"]
     @test split(str, ".:.") == ["a","ba.",".cba",":.dcba",""]
     @test split(str, r"\.(:\.)+") == ["a","ba.",".cba","dcba",""]
-    @test split(str, r"\.(:\.)+"; keep=false) == ["a","ba.",".cba","dcba"]
+    @test split(str, r"\.(:\.)+"; keepempty=false) == ["a","ba.",".cba","dcba"]
     @test split(str, r"\.+:\.+") == ["a","ba","cba",":.dcba",""]
-    @test split(str, r"\.+:\.+"; keep=false) == ["a","ba","cba",":.dcba"]
+    @test split(str, r"\.+:\.+"; keepempty=false) == ["a","ba","cba",":.dcba"]
 
     @test rsplit(str, ".:.") == ["a","ba.",".cba.:","dcba",""]
-    @test rsplit(str, ".:."; keep=false) == ["a","ba.",".cba.:","dcba"]
+    @test rsplit(str, ".:."; keepempty=false) == ["a","ba.",".cba.:","dcba"]
     @test rsplit(str, ".:."; limit=2) == ["a.:.ba..:..cba.:.:.dcba", ""]
     @test rsplit(str, ".:."; limit=3) == ["a.:.ba..:..cba.:", "dcba", ""]
     @test rsplit(str, ".:."; limit=4) == ["a.:.ba.", ".cba.:", "dcba", ""]