Skip to content

Conversation

@lukaszsamson
Copy link
Contributor

This PR:

  • Replaces keyword/term with concrete keyword lists in specs for all functions taking options argument
  • Adds missing documentation for some of the supported options
  • Adds specs for functions with options argument

Rationale:

  • Better LSP completions. ElixirLS is able to parse specs and provide completions for keyword options
  • Dialyzer validation
  • Easier machine transformation to elixir type annotation
  • Better consistency. A big part of the API surface already had typed keyword lists.

I originally planned to do this only for public functions, but I realized that it would be better to do it for all functions taking options argument. Some of the internal ones were already documented or had specs.

…s taking options argument

Add missing documentation for some of the supported options
Add specs for functions with options argumnet
@type nodata :: {:error, term} | :eof
@type chardata :: String.t() | maybe_improper_list(char | chardata, String.t() | [])

@type inspect_opts :: [
Copy link
Member

Choose a reason for hiding this comment

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

Should we reuse Inspect.Opts.new_opts here? If not, let's make sure to remove the deprecated options too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

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

Beautiful, just one tiny comment and we are good to go!

@josevalim josevalim closed this Jun 30, 2025
@josevalim josevalim reopened this Jun 30, 2025
@josevalim
Copy link
Member

CI is failing, can you please take a look? Please rebase if necessary. :)

@josevalim josevalim merged commit e3f7759 into elixir-lang:main Jun 30, 2025
12 of 13 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

Copy link
Contributor

@michallepicki michallepicki left a comment

Choose a reason for hiding this comment

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

Client requests a takeover.
"""
@spec take_over(binary, iodata, keyword) ::
@spec take_over(binary, iodata, take_over_opts) ::
Copy link
Contributor

Choose a reason for hiding this comment

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

This spec needs to cover options passed in from lib/iex/lib/iex/pry.ex:40 , so binding, dot_iex, env and stacktrace

* `:collapse_structs` - do not show struct fields that match
their default type
"""
@spec to_quoted(term(), to_quoted_opts) :: String.t()
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy-paste mistake, this should be a spec for to_quoted_string

[{Path.t(), Path.t()}],
atom(),
atom(),
compile_opts,
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to support opts passed from compile.yecc.ex and compile.leex.ex, I think all_warnings and verbose are currently missing

@josevalim
Copy link
Member

Thank you, I am pushing some fixes. A couple of those changes are also pass through and we were limiting the interface. I already had some WIP locally, I will include your feedback as I wrap up.

ggVGc pushed a commit to ggVGc/elixir-verbatim that referenced this pull request Sep 12, 2025
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