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

Inline Table Support #70

Closed
ttacon opened this issue Feb 9, 2015 · 21 comments
Closed

Inline Table Support #70

ttacon opened this issue Feb 9, 2015 · 21 comments

Comments

@ttacon
Copy link
Contributor

ttacon commented Feb 9, 2015

Hey, forgive me if I missed this. Recently, inline tables made it into the spec, is this on the roadmap for being supported in this package soon (meaning what kind of priority would supporting inline tables be, if it's one at all)? Just wondering! Also, awesome library!

@BurntSushi
Copy link
Owner

Yes absolutely. This tracks the TOML spec (as does BurntSushi/toml-test).

I have a very limited number of hours (if that) each day to work on side projects such as these, but certainly, anything that causes an incompatibility between this repo and the TOML spec should be a priority.

Pull requests are always welcome, but I'll eventually get to it if nobody else does.

@ttacon
Copy link
Contributor Author

ttacon commented Feb 9, 2015

Cool, I'm in a similar state - not enough time in the day for the side projects. If I can find some time though, I'll try to get to this - it's something I need for a side project.

@BurntSushi
Copy link
Owner

@ttacon Ah neat, that'd be awesome. My hope is that this can be done completely inside the lexer, which is reasonably simple once you've stared at it long enough (and with a good understanding of the TOML spec in your head). My fear is that this is wishful thinking and that the change will permeate into the parser, in which case, it will be quite a bit more hairy to implement.

@ttacon
Copy link
Contributor Author

ttacon commented Feb 9, 2015

So adding it to the lexer took all of 2 minutes (the lexer was super nice to read 👍). The parser, however, wasn't too happy with the changes... I'm going to try to make any changes minimal, but I probably won't have time to really dig into this until later unfortunately.

@BurntSushi
Copy link
Owner

That's awesome. If you want to put progress on to Github, I'll happily pull and piggy back off of you (if I get to it before you).

@ttacon
Copy link
Contributor Author

ttacon commented Feb 9, 2015

Just put the lexer changes I'd made at PR #71.

@ttacon
Copy link
Contributor Author

ttacon commented Feb 10, 2015

Hey, I just put up some more code to make parsing inline tables functional. However, I didn't spend enough time with parser.go to really understand what other information needs to be relayed (such as metadata about the file/keys/types/etc that this package keeps track of). TBH, I also wasn't sure what metadata you'd like extracted. Let me know what you think, I might have some more time this week to spend more time looking at it.

@ttacon
Copy link
Contributor Author

ttacon commented Feb 13, 2015

@BurntSushi Have you had any time to think about what metadata should be included in this (with respect to inline tables)?

@ttacon
Copy link
Contributor Author

ttacon commented Feb 17, 2015

@BurntSushi ^ bumpety bump :)

@BurntSushi
Copy link
Owner

@ttacon I'm sorry, but I'm still in the middle of it. I'm not sure when I'll be up for air. I haven't looked at your patch yet, so I'll just spit out a few things about meta data that I think I know.

The table {a = {b = 5}} should have precisely the same meta data as

[a]
b = 5

My hope is that your changes would happen at the lexer level and expose the table via itemTableStart and itemTableEnd. That way, the parser wouldn't have to change. But it looks like you added new items, which means the parser has to handle them.

Hopefully that helps some.

@ttacon
Copy link
Contributor Author

ttacon commented Feb 17, 2015

No problem! Just was wondering if you had seen the updates or not! I'll take another look at just making the changes in the lexer! Hopefully your research isn't keeping you too busy!

@ttacon
Copy link
Contributor Author

ttacon commented Feb 17, 2015

Just pushed some changes to make metadata parsing work, and added a test to decode_test. Let me know if that's what you had in mind.

@gutenye
Copy link

gutenye commented Apr 11, 2015

any updates? inline-table is really a cool feature.

@jleclanche
Copy link

Anyone working on this?

@ttacon
Copy link
Contributor Author

ttacon commented Jan 16, 2016

Ya, there's been an open PR in #71 for a while, just hasn't been merged.

@jleclanche
Copy link

@BurntSushi At a quick glance the code in #71 looks good. Could you review it?

@cespare cespare added this to the 0.4.0 milestone Feb 23, 2016
@s7v7nislands
Copy link

any update?

@davecthomas
Copy link

+1. Looks like they're getting close...

@silasdavis
Copy link

Looking at that PR looks like it will support parsing inline tables from toml, but not generating them. I would like the ability to encode certain values within a toml configuration to be represented as inline tables. How difficult would this be to implement, is it something that would be accepted? I would anticipate implementing it by configuring the encoder to treat certain keys to be rendered as inline tables. I think it would be reasonable to enable inline-table style rendering recursively for all sub-tables of a specified table.

@binary132
Copy link
Contributor

binary132 commented Feb 27, 2017

@silasdavis @BurntSushi -- What about using a struct field tag, as in:

type Foo struct {
    Bar map[string]string `toml:"some_name,inline"`
}

to cause toml.Marshal to render an inline table? Seems a bit of an edge case which field tags might be a good fit for.

@cespare
Copy link
Collaborator

cespare commented Mar 7, 2017

We now support decoding inline tables so this issue is resolved (and our TOML 0.4.0 support is complete).

I opened #160 for encoding inline tables.

@cespare cespare closed this as completed Mar 7, 2017
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

No branches or pull requests

9 participants