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

Try to clarify split docs #26634

Merged
merged 2 commits into from
Mar 28, 2018
Merged

Try to clarify split docs #26634

merged 2 commits into from
Mar 28, 2018

Conversation

simonbyrne
Copy link
Contributor

The description of the options was confusing.

We might also want change the keep option to something more descriptive, like keepempty

The description of the options was confusing.

We might also want change the `keep` option to something more descriptive, like `keepempty`
@simonbyrne simonbyrne added the docs This change adds or pertains to documentation label Mar 27, 2018
@@ -239,17 +239,20 @@ function rpad(
end

"""
split(s::AbstractString; limit::Integer=0, keep::Bool=false)
split(s::AbstractString, [chars]; limit::Integer=0, keep::Bool=true)
Copy link
Member

Choose a reason for hiding this comment

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

The brackets around chars can be removed with this clarification.

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
`chars` argument, `true` with `chars.
Copy link
Member

Choose a reason for hiding this comment

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

Missing a close ` at the end

@fredrikekre
Copy link
Member

We might also want change the keep option to something more descriptive, like keepempty

I think this would be good. I was confused by this the other week since I just assumed that keep=true would keep the char where we split like e.g. eachline(..., keep=true) keeps the newline char.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants