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

Set mapping keys to "decoded" when custom unmarshaler is used #426

Merged
merged 1 commit into from
Mar 17, 2025

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Sep 24, 2024

decode: set all keys under a mapping with custom marshaler
decoded

This is a naive fix for the issue of custom unmarshal and marking
keys as "decoded". It simply assumes that if there was a custom
unmarshal for a mapping type all keys got handlded by the custom
unmarshal code.

This might be too naive but it seems reasonable and it also seems
we would need a richer interface for a custom unmarshal that gives
access to MetaData if we want to be more fine grained.


decode: add (failing) test for undecoded fields in a struct

This commit adds a failing test for the scenario decribed in issue
#425

@arp242
Copy link
Collaborator

arp242 commented Sep 25, 2024

This seems like a reasonable change.

Could you add a more extensive test though? At the very least something like:

a   = 1
arr = [2]

[tbl]
b = 3

inline = {c = 4}

Just so that keys in different locations are tested: array, table, inline table. Maybe a few more? I'm not sure if these are different code-paths off-hand, but doesn't hurt to test it.

@mvo5
Copy link
Contributor Author

mvo5 commented Sep 26, 2024

Thanks a lot for your feedback! I updated the code and it now includes the more complex testcase that you suggested and that uncovered missing handling of nested which I added now too. I am happy to add more tests, but I need to think a bit what else to check for (I'm not super familiar with the toml details). Ideas welcome here of course :)

Please also let me know if I should squash or rebase my commits, I did them linearly mostly to make it easy to review but happy to do whatever is needed/preferred.

@mvo5 mvo5 changed the title [RFC] Set mapping keys to "decoded" when custom unmarshaler is used Set mapping keys to "decoded" when custom unmarshaler is used Sep 27, 2024
@mvo5
Copy link
Contributor Author

mvo5 commented Oct 8, 2024

Friendly ping, anything I can do or improve to help this along?

@arp242
Copy link
Collaborator

arp242 commented Oct 8, 2024

I'd like to carefully look at it and play around with it a bit. Not terrible time-consuming, but I don't have a lot of bandwidth for that right now. It's "on the list" and not forgotten, but it might be a bit before I have a chance to properly look at it.

Verified

This commit was signed with the committer’s verified signature.
arp242 Martin Tournoij
Keys were marked as "Undecoded" in the meta when using UnmarshalTOML
from the Unmarshaler interface. Unlike Primitive, I don't think this
makes sense as UnmarshalTOML() explicitly "decodes" the value.

Also mark all values below the table or array, and assume the parent
UnmarshalTOML() handled this correctly.

Fixes BurntSushi#425
@arp242 arp242 force-pushed the custom-decode-undecoded-fields branch from 8ff4571 to 66bf16f Compare March 17, 2025 09:33
@arp242 arp242 merged commit 6f7689d into BurntSushi:master Mar 17, 2025
7 checks passed
@mvo5
Copy link
Contributor Author

mvo5 commented Mar 17, 2025

Thank you so much! ❤️

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

2 participants