Skip to content

Comments

Allow InlineTable insert API to accept Into<Value>#1069

Merged
epage merged 1 commit intotoml-rs:mainfrom
terror:toml-edit-insert
Dec 5, 2025
Merged

Allow InlineTable insert API to accept Into<Value>#1069
epage merged 1 commit intotoml-rs:mainfrom
terror:toml-edit-insert

Conversation

@terror
Copy link
Contributor

@terror terror commented Nov 19, 2025

This diff updates InlineTable::insert to accept any V: Into<Value> just like get_or_insert, converting internally before storing. This lets callers pass primitive literals (&str, numbers, etc.) without manual Value construction while preserving existing behavior for Value. There are no behavior changes beyond the added flexibility.

@coveralls
Copy link

coveralls commented Nov 19, 2025

Pull Request Test Coverage Report for Build 19515719993

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 69.841%

Totals Coverage Status
Change from base Build 19044047551: 0.0%
Covered Lines: 6806
Relevant Lines: 9745

💛 - Coveralls

@terror terror changed the title Allow InlineTable insert APIs to accept Into<Value> Allow InlineTable insert API to accept Into<Value> Nov 19, 2025
@terror
Copy link
Contributor Author

terror commented Dec 4, 2025

@epage Was there still a blocker for this? I'd be happy to rebase and amend the commit message if that's needed.

@epage
Copy link
Member

epage commented Dec 5, 2025

Every time I look at this, I go back and forth. We don't really have a good principled reason for what we do. I'll go ahead and merge and we can re-evaluate when we change up tables (#762 (comment)).

@epage epage merged commit 18cee6b into toml-rs:main Dec 5, 2025
15 checks passed
@terror terror deleted the toml-edit-insert branch December 5, 2025 20:48
@epage
Copy link
Member

epage commented Dec 6, 2025

The release was yanked and the change reverted because it broke type inference, see #1074

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.

3 participants