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

Added an accessible array of field names to DocumentedFields #15

Merged
merged 4 commits into from
Oct 29, 2024

Conversation

compilin
Copy link
Contributor

Hello, I found your crate while looking for a way to make a struct to deserialize a config file, that I could automatically generate a documentation for said file from by using its fields' docs. Your crate was very close to allowing that but still required to hardcode the field names in order to access their documentation. I thought that since it used those field names already, it would be trivial to store them alongside the docs and make them accessible. And indeed it was.

I don't know if anyone else needs this feature. I would have thought so but I'm surprised by the lack of result while searching for it. Regardless, as far as I can tell this PR doesn't induce any backwards-incompatible changes, so might as well share my patch.

I chose to account for unnamed fields by just omitting them from the array, simplifying access but breaking the field name <=> doc index match (i.e you can't just zip() them), on account of the fact that you can always use get_field_docs to map name to docs. If you prefer an alternate implementation (array of Optionals, empty names or other sentinel value for unnamed fields, etc…) I don't mind adjusting it.

I can also try and put this behind a feature flag if you prefer.

@cyqsimon
Copy link
Owner

Yeah I'm open to adding this. Code looks good.

I'm curious though, how are you implementing your deserialisation? Because I recently did something very similar, and there was indeed a bit of manual fidgeting involved. Granted it was mostly due to my multi-layered struct shenanigans, but it would be nice if I can take inspiration from someone else's approach to the same problem.

@cyqsimon cyqsimon merged commit 9c224d0 into cyqsimon:master Oct 29, 2024
4 checks passed
@compilin
Copy link
Contributor Author

Thanks for the merge !

You mean how I plan to use those field names ? Well I thought about producing a "template" config file with comments like your example here, except in my case it's JSON so I can't really do that. Instead I think I'll just have a CLI flag to generate an empty config file, and another flag to list the fields and their associated documentation.

… I could have just serialized my struct to a Map and taken the keys from that, similarly to how you've done it. But at least this way it's more easily accessible and compatible with renames I guess.

@cyqsimon
Copy link
Owner

Instead I think I'll just have a CLI flag to generate an empty config file, and another flag to list the fields and their associated documentation.

Yeah that makes sense. Sounds like a pretty decent choice in lieu of comments. Speaking of which, you may be interested in checking out JSON5, which is quite a bit more suitable for a human-oriented config file.

… I could have just serialized my struct to a Map and taken the keys from that, similarly to how you've done it. But at least this way it's more easily accessible and compatible with renames I guess.

I concur; your approach is probably slightly more accurate (whatever that means). Thanks for sharing!

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.

2 participants