Bug in writing out file from generic table, keys placed in wrong section

[jgriffitts]

There appears to be an issue with generic tables when an Add() is used to append a key/value pair to a lower level after subtables/sections have been added. The following C# program exhibits the problem:

    class Program
    {
        static void Main(string[] args)
        {
            var tbl = Toml.Create();
            tbl.Add("root1", 1);    // These three keys added at root level
            tbl.Add("root2", 2);
            tbl.Add("root3", 3);

            var t2 = tbl.AddTomlObject("Level2", Toml.Create());
            t2.Add("second1", "x");
            t2.Add("second2", "y");

            tbl.Add("root4", 4);    // Added at root level but spuriously written into [Level2.Level3]

            var t3 = t2.AddTomlObject("Level3", Toml.Create());
            t2.Add("third1", "a");
            t2.Add("third2", "b");

            t2.Add("second3", "z"); // Added to [Level2] but spuriously written into [Level2.Level3]
            tbl.Add("root5", 5);    // Added at root level but spuriously written into [Level2.Level3]

            var result = Toml.WriteString(tbl);
            Console.WriteLine(result);
        }
    }

The TOML output from this program is:

root1 = 1
root2 = 2
root3 = 3

[Level2]
second1 = "x"
second2 = "y"

[Level2.Level3]
third1 = "a"
third2 = "b"
second3 = "z"
root4 = 4
root5 = 5

The second3, root4, and root5 keys in the last three lines of the output are misplaced! They appear as part of [Level2.Level3] but were added into [Level2] and at root level. The keys root4 and root5 should appear immediately after root3, and second3 should be after second2.

It appears to me that the easiest fix is in Nett\writer\TomlTableWriter.cs in method WriteTableRows() and WriteNormalTomlTable(), by making it write all the non-table rows first, then the table rows. I have tried a fix along these lines, by taking two passes through the table rows. The single foreach loop has been expanded to two foreach loops, each iterating through all rows but qualifying the write with an if:

        private void WriteTableRows(string parentKey, TomlTable table)
        {
            Assert(table != null);

            foreach (var r in table.InternalRows)
            {
                // Write out all the non-table rows first to get all of the
                //  root-level keys. These cannot be written after the
                //  table section lines.
                if (r.Value.TomlType != TomlObjectType.Table)
                {
                    this.WriteTableRow(parentKey, r);
                }
            }

            foreach (var r in table.InternalRows)
            {
                if (r.Value.TomlType == TomlObjectType.Table)
                {
                    this.WriteTableRow(parentKey, r);
                }
            }
        }

Similar change appears to be needed in WriteNormalTomlTable(), taking two passes through the rows:

        private void WriteNormalTomlTable(string parentKey, TomlKey key, TomlTable table)
        {
            this.WritePrependComments(table);
            this.writer.Write('[');
            this.writer.Write(parentKey + key);
            this.writer.Write(']');
            this.writer.WriteLine();
            this.WriteAppendComments(table);

            // Write out all the non-table rows first to get all of the
            //  root-level keys. These cannot be written after the
            //  table section lines.
            foreach (var r in table.InternalRows)
            {
                if (r.Value.TomlType != TomlObjectType.Table)
                {
                    this.WriteTableRow(CombineKey(parentKey, key), r);
                }
            }

            foreach (var r in table.InternalRows)
            {
                if (r.Value.TomlType == TomlObjectType.Table)
                {
                    this.WriteTableRow(CombineKey(parentKey, key), r);
                }
            }
        }

I don't have high confidence in this fix, but it seems to work with my test cases. The other approach would be to modify Add() so that it rearranges the table when root keys are added to insert them before any subtables. That seems more difficult.

Note: I don't have a deep understanding of Nett and the operations on generic tables are not well documented, so it's possible that I am using it incorrectly.

[paiden]

Thx for the bug report. You analysis was correct. This was something I missed because personally I always use the class => TOML serialization mechanism that created TOML tables where the tables/table arrays were always added last. I simply missed that users may not add objects in that order also.

I made a fix quite similar to the one described, but inside the TOML table.

[jgriffitts]

Your fix is better than mine. Mine fails when arrays of tables are present, but yours seems to cover that case.

Thanks!