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

Elixir's Kernel defines both to_char_list and to_charlist #4909

Closed
Qqwy opened this issue Jun 29, 2016 · 12 comments
Closed

Elixir's Kernel defines both to_char_list and to_charlist #4909

Qqwy opened this issue Jun 29, 2016 · 12 comments

Comments

@Qqwy
Copy link
Contributor

Qqwy commented Jun 29, 2016

Current behavior

Elixir's Kernel defines the macro to_charlist/1 as well as to_char_list/1. The first version includes documentation, the second version does not.

Expected behavior

I am not sure if there is a reason for the second version (to_char_list/1) to exist.

  • If it is a duplicate, we could remove it.
  • If however there is a reason for its existence, it should receive documentation.
@Qqwy
Copy link
Contributor Author

Qqwy commented Jun 29, 2016

Hmm... Diving in the source, I see that it already is marked as 'TODO: Deprecate in version 1.5', so maybe I spoke too soon and this issue is already kown.

Right now it has @doc false. Is it better to give it documentation that states that it is deprecated as well?

@whatyouhide
Copy link
Member

The second version (to_char_list/1) will be deprecated in future versions of Elixir 1.5, and for now it's softly deprecated, meaning it's not shown in documentation but no warning is emitted when it's used. You can see TODO: ... deprecation comments here.

We shouldn't show documentation for these functions, so that users will just not see them as available.

@michalmuskala
Copy link
Member

If it's soft-deprected, should that mean, it shouldn't show up when TAB-completing in iex? It currently does show up.

That's contrary to String.strip/2 that was deprecated as well, but does not show up when TAB-completing.

@eksperimental
Copy link
Contributor

eksperimental commented Jun 29, 2016

When i read this issue, i thought of thw same thing michal is mentioning. How did qqwy find out about the function if he hadn't read the source code by then. And IEx came to my mind

@whatyouhide
Copy link
Member

Oh, @michalmuskala is right, it appears in IEx. This appears to be a problem just in Kernel though, since if you try to autocomplete String.to_char it only shows String.to_charlist/1 as expected. Kernel however behaves differently, i.e., Kernel.to_char and just to_char both show to_char_list/1 as well.

@lexmag
Copy link
Member

lexmag commented Jul 1, 2016

Should be fixed in 9740544. :bowtie:

@whatyouhide
Copy link
Member

I definitely missed that it was commented D: Thanks @lexmag!

@eksperimental
Copy link
Contributor

thank you @lexmag for the fix, it was my fault

@josevalim
Copy link
Member

And of everyone who reviewed it, like me. :) it happens!

On Saturday, July 2, 2016, Eksperimental notifications@github.com wrote:

thank you @lexmag https://github.com/lexmag for the fix, it was my fault


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#4909 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAAlbt7ja7-8OVf5eerUcuUjIGD6ncptks5qRqO6gaJpZM4JBHaw
.

José Valim
www.plataformatec.com.br
Skype: jv.ptec
Founder and Director of R&D

@eksperimental
Copy link
Contributor

@josevalim I think we should backport this to 1.3.x

@eksperimental
Copy link
Contributor

nevermind, @lexmag already took care of it cc35c8f

@Qqwy
Copy link
Contributor Author

Qqwy commented Jul 3, 2016

Guys, you are awesome 😍

jmitchell added a commit to jmitchell/elixir-school that referenced this issue Dec 17, 2016
Now it's best to use `to_charlist/1` since `to_char_list/1` is being
deprecated (see elixir-lang/elixir#4909).
doomspork pushed a commit to elixirschool/elixirschool that referenced this issue Dec 18, 2016
Now it's best to use `to_charlist/1` since `to_char_list/1` is being
deprecated (see elixir-lang/elixir#4909).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants