Skip to content

Update String.split/2 into String.split/3 #1768

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

Closed
wants to merge 1 commit into from
Closed

Conversation

zorn
Copy link

@zorn zorn commented Aug 31, 2024

There is no String.split/2, best I can tell.

https://hexdocs.pm/elixir/String.html#split/3

But String.split/3 does behave as the blog post explains.

iex(1)> String.split("")
[]
iex(2)> String.split("", ",")
[""]

There is no `String.split/2`, best I can tell.

https://hexdocs.pm/elixir/String.html#split/3

But `String.split/3` does behave as the blog post explains.

```
iex(1)> String.split("")
[]
iex(2)> String.split("", ",")
[""]
```
@josevalim
Copy link
Member

That's a bug in Elixir. I will take a look at it. :) Thanks for noticing.

@sabiwara
Copy link
Contributor

sabiwara commented Aug 31, 2024

There is no String.split/2, best I can tell.

Actually there is a String.split/2, which corresponds to the default options \\ []. Hex docs shows it properly, and as you mentioned yourself you are able to call String.split("", ",") 🙂

The purpose of defaults is precisely to define several arities with one signature.

@sabiwara
Copy link
Contributor

iex(1)> String.split("")
[]

That's a bug in Elixir.

@josevalim I agree this is inconsistent with String.split/2/3, but it seems intentional looking at the tests and code.

Removing the trim seems like a breaking change, should we document the trimming behavior instead?

@sabiwara
Copy link
Contributor

Random thought: we could change String.split/2/3's pattern to pattern() | Regex.t() | :whitespace, deprecate String.split/1, and String.split(s) would have to be replaced by String.split(s, :whitespace, trim: true).
This could also expose whitespace -- non_breakable and make it work with more options?

@josevalim
Copy link
Member

Yes, agreed, this is intentional.

When it comes to provide a better API, a general golden rule is to not have the return type changed by options. So the real issue here is that the return type depends on the option trim: true | false. So what we would want to do in the future is to deprecate the trim option and introduce split_and_trim, which may return an empty list.

:whitespace could be a choice if we want to allow whitespace to be used with other options.

But for now, I think we are good, especially because String.split/2 does behave exactly as described in the post. :)

@zorn
Copy link
Author

zorn commented Sep 1, 2024

The purpose of defaults is precisely to define several arities with one signature.

Interesting. This is kind of a new concept for me. In the past, if a function accepted three arguments, even if the third one was optional, I would still always use the /3 declaration.

But as you say, because the third argument is optional, the language infers that there is a /2 version.

As seen trying to tab complete the function name in an iex session:

iex(5)> String.split
split/1       split/2       split/3       split_at/2    splitter/2    splitter/3

Thanks for the quick responses and guidance. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants