Skip to content

feat(writer): Implement TomlInteger#1004

Merged
epage merged 6 commits into
toml-rs:mainfrom
pastelmind:toml-writer-integer
Jul 11, 2025
Merged

feat(writer): Implement TomlInteger#1004
epage merged 6 commits into
toml-rs:mainfrom
pastelmind:toml-writer-integer

Conversation

@pastelmind

@pastelmind pastelmind commented Jul 11, 2025

Copy link
Copy Markdown
Contributor

Implements TomlInteger and TomlIntegerFormat following the discussion at #812. This supersedes #837.

This design should be extensible in case we later decide to support integer separators, custom hexadecimal case, or padding (leading zeroes).

Comment thread crates/toml_writer/src/integer.rs Outdated
Comment thread crates/toml_writer/src/integer.rs Outdated
Comment thread crates/toml_writer/src/integer.rs Outdated
@coveralls

coveralls commented Jul 11, 2025

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 16226589326

Details

  • 41 of 68 (60.29%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.07%) to 69.558%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/toml_writer/src/integer.rs 34 61 55.74%
Totals Coverage Status
Change from base Build 16223185703: -0.07%
Covered Lines: 6624
Relevant Lines: 9523

💛 - Coveralls

Comment thread crates/toml_writer/src/integer.rs Outdated
Comment thread crates/toml_writer/tests/integer.rs
- Remove unused _separators field in TomlIntegerFormat
- Change as_decimal() et al to accept mut self
- TomlInteger contains TomlIntegerFormat
@pastelmind pastelmind requested a review from epage July 11, 2025 17:31
@pastelmind

Copy link
Copy Markdown
Contributor Author

I'm not familiar with snapbox, so feel free to suggest a different test setup.

@epage

epage commented Jul 11, 2025

Copy link
Copy Markdown
Member

tests are failing when --no-default-features is applied

@pastelmind

Copy link
Copy Markdown
Contributor Author

Sorry, it should run fine now

@epage epage merged commit 4741561 into toml-rs:main Jul 11, 2025
15 checks passed
@epage

epage commented Jul 11, 2025

Copy link
Copy Markdown
Member

For future reference, I prefer people to maintain a PR's commits for how they would like them to look when merged, rather than a commit per PR update. There is value in having and merging multiple commits in a PR that can get lost with squashing.

@pastelmind pastelmind deleted the toml-writer-integer branch July 11, 2025 18:21
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