toml-lang / toml

Tom's Obvious, Minimal Language
https://toml.io
MIT License
19.45k stars 847 forks source link

Table section editing inline table in inline array #908

Closed brandonchinn178 closed 2 years ago

brandonchinn178 commented 2 years ago

The spec says:

Any reference to an array of tables points to the most recently defined table element of the array.

which means that this is currently valid (verified at http://toml-online-parser.ovonick.com/):

a = [{ b = 1 }]
[a.c]
foo = 1
{
    "a": [
        {
            "b": {"type" : "integer", "value" : "1"},
            "c": {
                "foo": {"type" : "integer", "value" : "1"}
            }
        }
    ]
}

which seems a bit inconsistent since extending inline tables in general is invalid:

a = { b = 1 }
[a.c]
foo = 1

Is this intentional?

marzer commented 2 years ago

array of tables

"Array-of-tables" in this context does not mean any normal array containing inline tables (as in your example), but a specific language construct for representing table elements of arrays using the [[array.of.tables]] syntax. To re-write your example using what the spec document actually means, it would be this:

[[a]]
b = 1

[a.c]
foo = 1

The second header, [a.c], is in effect actually [a[0].c] (since a is an array-of-tables with only one element, which was the most recent one named in the document) - adding c to a[0] in this fashion is fine since it is a regular table, not an inline one.

marzer commented 2 years ago

Also, your original example that you've verified:

a = [{ b = 1 }]
[a.c]
foo = 1

Is very much not legal - a is an array, it can't be 'appended' to as if it were a table. The online parser you've chosen would fail the toml-test tests methinks (it was last updated in 2016!).

The key takeaway is that the two things known as "Arrays" and "Arrays-of-Tables" in the TOML spec are two totally different things with very different semantics.

brandonchinn178 commented 2 years ago

The online parser you've chosen would fail the toml-test tests methinks.

Nope! 😄 To be precise, the online parser I linked would probably fail the tests, yes, but my parser passes the entire toml-test suite and it successfully parses this. Sounds like I should make an issue in toml-test?

marzer commented 2 years ago

but my parser passes the entire toml-test suite and it successfully parses this. Sounds like I should make an issue in toml-test?

Oh, exciting! Yeah. Sounds like a great idea to me 😄

arp242 commented 2 years ago

Tests for appending to existing tables were added a few months ago, but I don't think there are any tests for appending to existing arrays (I think, unless I'm, misremembering). I'll have a look later, if someone doesn't send in a patch first (bit busy this week).

brandonchinn178 commented 2 years ago

tests for appending to existing arrays

There is a test for appending to inline arrays: https://github.com/BurntSushi/toml-test/blob/master/tests/invalid/array/tables-1.toml

There isn't a test for extending tables within inline arrays.

pradyunsg commented 2 years ago

Closing this out, since the discussion seems to have run its course.