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

Support encoding comments in toml tag #97

Closed
wants to merge 4 commits into from

Conversation

echlebek
Copy link

@echlebek echlebek commented Oct 6, 2015

Implements the feature suggested in #75. I'm not sure this is actually a good idea, but thought I'd share anyway.

A possible issue with this is that people who use comments might experience breakage if later on the library implements an option that matches their exact comment. That seems like a pretty remote possibility to me, since comments are usually partial sentences.

Another way to accomplish encoding comments would be the way encoding/xml does it, but I'm not particularly fond of that approach either tbh.

This implements the suggestion in the referenced issue.
I wouldn't say this is a particularly nuanced way to support
encoding comments, but it was easy to implement and I wanted
to see if people would be receptive to it.
@DevelopSMM
Copy link

This branch works great for encoding. However, decoding fails on all fields containing a comment.

Adding comment support to struct tags broke decoding for those fields.
This commit fixes that bug, along with the failing test in the previous
commit.
@echlebek
Copy link
Author

@DevelopSMM thanks for the feedback. I've added a few commits that fix this problem.

I didn't bother rebasing or merging with tip, but would be happy to do so if there was interest in merging this branch.

@cespare
Copy link
Collaborator

cespare commented Apr 26, 2016

I lean towards not putting more things into the struct tags.

ebrady pushed a commit to dvln/toml that referenced this pull request Oct 24, 2016
@cespare
Copy link
Collaborator

cespare commented Mar 7, 2017

I don't think we should go down the road of encoding arbitrary comments in struct tags.

@cespare cespare closed this Mar 7, 2017
@ghost
Copy link

ghost commented Mar 7, 2017

Absolutely makes sense, but do you have any solutions on how to handle comments then? When working with config files it's nice to be able to document inline what each option is supposed to do.

@cespare
Copy link
Collaborator

cespare commented Mar 7, 2017

Comments are fully supported when decoding TOML. This PR is about encoding comments.

@echlebek
Copy link
Author

echlebek commented Mar 7, 2017

Yeah, this was never a good idea.

I think it's useful to be able to generate TOML with comments, (say for outputting default config files) but there's gotta be a better way. Maybe text/template.

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