Skip to content

Fix handling of inline tables #58

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.

Sign up for GitHub

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

Jump to bottom
Merged
bkchr merged 7 commits into from
Mar 6, 2025
Merged

Fix handling of inline tables #58

bkchr merged 7 commits into from
Mar 6, 2025

Conversation

@ijackson
Copy link
Contributor

@ijackson ijackson commented Mar 5, 2025

Fixes #57

I think this may have been broken as a side effect of 56a40f1 apparently released in 1.3.0 (!)

ijackson added 2 commits March 5, 2025 18:45
@ijackson
toml_edit tables: Use TableLike and .as_table_like
51e8f0d 
Fixes bkchr#57
@ijackson
toml_edit tables: clippy.toml forbidden methods
5ce9214 
I used this file and
   clippy -- -A clippy::all -D clippy::disallowed_methods
to find the places I needed to fix.
bkchr
bkchr reviewed Mar 5, 2025
View reviewed changes
Copy link
Owner

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Ty! Would it be possible to get a test for this?

ijackson added 3 commits March 6, 2025 11:46
@ijackson
fixup! toml_edit tables: clippy.toml forbidden methods
a138066
@ijackson
CI: check that we don't call toml_edit::Item::as_table
fe807ed 
This will stop this bug sneaking back in.

Doing this here saves having to duplicate all the tests with versions
that use inline tables instead.  (We'll have one test, though.)
@ijackson
tests: One test of an inline table
1ded013
@ijackson
Copy link
Contributor Author

ijackson commented Mar 6, 2025 •
edited
Loading

I've added a single test case, which does indeed fail now for me locally if I revert "toml_edit tables: Use TableLike and .as_table_like".

I considered how to prevent calls to .as_table sneaking back in. I don't think duplicating all the tests (or having some magic in the test that converts the TOML) is sensible. Instead, I have tried to add a CI job which uses clippy to detect this.

I don't have much experience with github CI (I mostly work with gitlab) so I hope what I've done (a) looks sane and (b) works.

bkchr
bkchr approved these changes Mar 6, 2025
View reviewed changes
@bkchr
Copy link
Owner

bkchr commented Mar 6, 2025

I don't have much experience with github CI (I mostly work with gitlab) so I hope what I've done (a) looks sane and (b) works.

Ty! All looks good. :)

@ijackson
Copy link
Contributor Author

ijackson commented Mar 6, 2025

Thanks :-).

@bkchr bkchr merged commit 88f5bf0 into bkchr:master Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bkchr bkchr bkchr approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Failure to work with inline tables in Cargo.toml

2 participants

@ijackson @bkchr