kowainik / tomland

🏝 Bidirectional TOML serialization
https://kowainik.github.io/posts/2019-01-14-tomland
Mozilla Public License 2.0
120 stars 39 forks source link

tableMap Codec with implicit tables not parsed #336

Open everythingfunctional opened 4 years ago

everythingfunctional commented 4 years ago

I got a report from one of my users that the tableMap codec isn't working with implicit tables.

I did a little bit of playing with the examples/biTest.toml file and found that it still worked if I changed

[colours]
yellow = "#FFFF00"
red    = { red = 255, green = 0, blue = 0 }
pink   = "#FFC0CB"

to

[colours]
yellow = "#FFFF00"
pink   = "#FFC0CB"
[colours.red]
  red = 255
  green = 0
  blue = 0

still get output

[colours]
  pink = "#FFC0CB"
  yellow = "#FFFF00"

  [colours.red]
    blue = 0
    green = 0
    red = 255

but if I changed it to

[colours.red]
red = 255
green = 0
blue = 0

or

[colours.red]
    blue = 0
    green = 0
    red = 255

[colours.pink]
    red = 255
    green = 208
    blue = 220

then the output is just

[colours]

I'm guessing that the keys aren't combined into a single table without the single table declaration, and then the converter isn't picking them up. I'm a bit out of my depth on how to fix it though.

everythingfunctional commented 4 years ago

tried one more. this

[colours.red]
red = 255
green = 0
blue = 0

[colours]
yellow = "#FFFF00"
pink   = "#FFC0CB"

only gives output

[colours]
  pink = "#FFC0CB"
  yellow = "#FFFF00"
chshersh commented 4 years ago

Hi @everythingfunctional! Thanks for reporting the problem with the specific use-case. tomland indeed doesn't handle implicit tables in the same way as explicit at the moment. The problem is that during parsing we don't keep information about whether the table is explicit or implicit. @vrom911 and I tried working on a patch for this problem but turns out that it requires a lot of work and massive changes to internal TOML AST representation. Changing TOML AST will have some other benefits as well, but it's a lot of work. It's definitely on our roadmap but it will take some time.

Current workaround is to specify implicit tables explicitly. Sorry for the inconvenience.

everythingfunctional commented 4 years ago

I had it feeling it might be complicated. Glad to hear it's at least in the plan. Thanks for looking into it.