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

feat: Add support for float and boolean hash keys #650

Merged

Conversation

MartinHowarthFlexciton
Copy link
Contributor

The unreachable! code at https://github.com/rust-cli/config-rs/blob/main/src/file/format/yaml.rs#L53 is reachable, and this PR fixes it for two simple types: reals and booleans.

Prior to this change you'd get this error trying to parse the following:

thread '...' panicked at .../config-0.15.9/src/file/format/yaml.rs:53:26:
internal error: entered unreachable code

The problematic case for me turned out to be float keys:

inner_float:
    0.1: "float 0.1"
    0.2: "float 0.2"

The change in this PR follows the same pattern as already exists for Strings and Integers - just parse them as strings and leave it up to the user to work out what to do with that.

Personally I only need the Real support, so happy to remove the Boolean case if that's at all contentious.

I decided not to look into whether you can craft a hash with the other Yaml types (below) because it's such an odd use case it didn't seam realistic! That said, #547 suggests that just treating Hash as string would have avoided that users' problem. However, converting from these types back into the exact string format they were parsed in would be difficult, so it's not clear what the right implementation would be. So getting the simpler case (reals, booleans) done feels like the right sized change to make.

    Array(Array),
    Hash(Hash),
    Alias(usize),
    Null,
    BadValue,

Let me know what you think! And thanks for maintaining this useful crate :)

@epage
Copy link
Contributor

epage commented Mar 10, 2025

Seems we should turn that panic into an error

@MartinHowarthFlexciton MartinHowarthFlexciton force-pushed the handle-float-and-bool-hash-keys branch from c00edcd to afba1e0 Compare March 11, 2025 11:30
@MartinHowarthFlexciton
Copy link
Contributor Author

I've turned the remaining unsupported cases into a useful error message as you suggested

@coveralls
Copy link

coveralls commented Mar 12, 2025

Pull Request Test Coverage Report for Build 13814392108

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 65.403%

Totals Coverage Status
Change from base Build 13814057810: 0.02%
Covered Lines: 949
Relevant Lines: 1451

💛 - Coveralls

Verified

This commit was signed with the committer’s verified signature. The key has expired.
danielleadams Danielle Adams
@MartinHowarthFlexciton MartinHowarthFlexciton force-pushed the handle-float-and-bool-hash-keys branch from 7915e23 to 1ecc731 Compare March 12, 2025 14:51
@epage epage merged commit aadd8c8 into rust-cli:main Mar 12, 2025
14 of 15 checks passed
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.

None yet

3 participants