Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Add formatters for Option, Map, Period, and Atoms #19658

Merged
merged 2 commits into from
Feb 13, 2024
Merged

Add formatters for Option, Map, Period, and Atoms #19658

merged 2 commits into from
Feb 13, 2024

Conversation

hamzaremmal
Copy link
Member

Before:

  • using a Map with the i-interpolator won't compile
  • using an Option in the i-interpolator will fall back to the Show[Product] given which results in a call to toString

dwijnand
dwijnand requested changes Feb 12, 2024
View reviewed changes
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

I don't think we should be using the i interpolator in writing the show instances.

@hamzaremmal
Copy link
Member Author

I don't think we should be using the i interpolator in writing the show instances.

I actually hesitated at the beginning because it shouldn't really a problem (but I agree on the premise)

@dwijnand
Copy link
Member

Could you pull my changes in #19674 (or approve the PR there)? Then we can merge one or the other PRs and get these changes finally in (I had these changes locally for a long time, but never had a strong reason to work them into a PR).

@hamzaremmal @dwijnand
Set various Show instances
b2b175c 
Co-authored-by: Dale Wijnand <dale.wijnand@gmail.com>
@hamzaremmal hamzaremmal mentioned this pull request Feb 13, 2024
Closed
dwijnand
dwijnand approved these changes Feb 13, 2024
View reviewed changes
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

Thank you!

@dwijnand dwijnand changed the title Add formatters for Option & Map Add formatters for Option, Map, Period, and Atoms Feb 13, 2024
@dwijnand dwijnand merged commit f92349b into scala:main Feb 13, 2024
16 checks passed
@Kordyjan Kordyjan added this to the 3.4.1 milestone Feb 14, 2024
@WojciechMazur WojciechMazur mentioned this pull request Jul 1, 2024
Merged
WojciechMazur added a commit that referenced this pull request Jul 2, 2024
@WojciechMazur
Backport "Add formatters for Option, Map, Period, and Atoms" to LTS (#…
0155d65 
…20915)

Backports #19658 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@dwijnand dwijnand dwijnand approved these changes

Assignees

@dwijnand dwijnand

Labels
None yet
Projects
None yet
Milestone
3.4.1
Development

Successfully merging this pull request may close these issues.
3 participants
@hamzaremmal @dwijnand @Kordyjan