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

Improve string.inspect for sets #786

Closed
wants to merge 1 commit into from

Conversation

malbarbo
Copy link
Contributor

@malbarbo malbarbo commented Jan 3, 2025

Now

set.from_list(["a", "b"]) |> string.inspect

produces

"set.from_list([\"a\", \"b\"])"

@malbarbo
Copy link
Contributor Author

malbarbo commented Jan 3, 2025

The code uses @internal, which requires gleam 1.1, so it is necessary to change gleam version or remove the internal annotation (The array_bit module has a inspect function).

@lpil
Copy link
Member

lpil commented Jan 3, 2025

There's no guarantee that a record with the tag "set" is a Set, so this may not be safe.

If you would like to make a change to Gleam please open an issue so we can agree on a design. Unplanned pull requests rarely get merged due to rework taking lots of time.

@malbarbo
Copy link
Contributor Author

malbarbo commented Jan 3, 2025

Thank you for the feedback! I’ll open an issue to discuss the design and address the concern about ensuring safety with records tagged as "set". I appreciate the guidance on the process and will align with it in the future. Thank you.

@malbarbo malbarbo closed this Jan 3, 2025
@lpil
Copy link
Member

lpil commented Jan 3, 2025

Thank you! 💜

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

Successfully merging this pull request may close these issues.

2 participants