Table key source

[vaartis]

Is your feature request related to a problem? Please describe.

Currently, table values all have sources attached to them (line and column), which is very useful. However, there seems to be no way of getting the source of the table key (the name = part).

Describe the solution you'd like

I'm not sure what the best interface for this would be, as this information would probably need to be accessible from the table itself and not from the node that is received with operator[] and such, since they don't store that they're in a table. Probably some method on toml::table like key_source(std::string) -> std::optional<toml::source_region> could suffice.

Additional context

Basically, since no libraries support preserving the structure of the file, i'm rolling my own solution that only replaces parts of the file by accesing the source of the node. It would be very useful to be able to also get the source of the key, so I could replace them easily without reformatting the whole table or doing it in roundabound ways.

[marzer]

Yah, you're right that it would need to be stored outside the values. I'd be wary against adding it to the tables since that's going to add quite a lot of extra state to each node (and force an ABI break); an alternative solution would be to provide an optional metadata 'container' class that could collect this sort of thing during parsing, e.g. something like:

# config.toml:
[foo]
bar = 5

toml::metadata md;
auto table = toml::parse_file('config.toml', &md);

std::cout << md.key_source(table["foo"]["bar"]) << "\n";

line 3, column 1 of config.toml

Now that I think of it, this would be a good backwards-compatible way of also implementing a solution to #28...

Only downside is that it means the source information would be split between being stored in the nodes (the existing .source()) and stored in an external metadata object, which I guess would be a bit weird without knowing the historical context of how it came to be that way.

[vaartis]

The outside container may be a good idea.. although i suppose that would imply that source() will have to be removed or something, but that will break compatibility.. perhaps instead it could be made to somehow point to the metadata object instead of storing data in the table itself (though this may be tedious and error prone due to having to ensure that the metadata container lives at least as long as the table itself...).

I can't say I have a good solution for this. The most obvious way is putting it into tables themselves and it's just no good..
One thing i worry about the "outside container" is that there needs to be a good way to get the data for the node however deep in the tree easily (if, for example, the path to the node is dynamic), otherwise it will be terrible to use. In the example you've shown the container is supposed to somehow figure out the path from the table value, even though the table value doesn't actually include a path to itself. So the container would probably have to either store some kind of ID for each node and use that, if it's to work like in this example. Either that or the node storing the path to itself, i guess, but that breaks the ABI(?). However if somehow this is made possible, it should work quite well, again if the usage would be as simple as in the example.

[vaartis]

I suppose I'll also point out in this issue about editing and preserving the formatting of the file, if anyone looking through the issues finds this.

The idea is to after reading the file, store it (or re-read it on save) and when serializing, look up positions in the source of the nodes and replace the data in the file with the new changed data. It probably could be implemented as a wrapper around the current parse_file system without touching it all, and incorporated into the library, if someone would be willing to do it (sadly my own implementation is ugly and oriented on serializing very concrete data).

[marzer]

... although i suppose that would imply that source() will have to be removed or something, but that will break compatibility

I won't be removing the existing source() since I don't want to break things for existing users. If I do a v3 of the library I'd happily break all sorts of things because there's a few early design mistakes I'd like to correct, but only when I do a major version bump. Until then it's backwards-compatibility for everyone.

... there needs to be a good way to get the data for the node however deep in the tree easily (if, for example, the path to the node is dynamic), otherwise it will be terrible to use.

Of course. The example above is to illustrate the concept, not an example of exactly how it would be implemented, so don't read into it too much beyond that.

Either that or the node storing the path to itself, i guess, but that breaks the ABI(?)

Yeah, if I add state to any existing classes that's an implict ABI break - function calls will still link just fine but the size/layout of the objects will change. The metadata object is a non-breaking solution (pre-v3) because it doesn't modify anything that's already being used in the wild.

[vaartis]

Seems like the metadata container is the most likely solution then, if compatibility is not to be broken.
Perhaps, as to not break source() for existing users, the parse_file with metadata could be a special overload that creates nodes that have source() pointing to the metadata container (as i've also suggested in the previous post) in addition to having the container itself to use (so the source() would not me completely useless on these nodes), and the old one would be left as-is. I, personally, would be happy with any solution, but of course I'm not the library maintainer. Your idea with the container seems like the most sane solution that should not break anything.

[marzer]

@vaartis If you're still interested in this feature I've pushed an implementation of it to the v3 branch. Since it's going to be an ABI-breaking release I decided to store the source info directly in the tables as part of the new toml::key class, replacing my use of plain std::string as the table's internal map key. It can be accessed anywhere you might have previously accessed the string keys (structured bindings, iterators, etc.).

I won't merge v3 into master for a while yet since there's more work to be done before I cook a new release, but I welcome you to try it before then.

The full list of changes in the v3 branch can be found here.

[vaartis]

Thank you. I'll try porting my code to v3 when I can

[vaartis]

Sorry, been a while.. Getting back to this, is there any way in the 3.1.0 version to get the actual toml::key from the table without iterating it? Basically, I just want to get the source data because I store paths as strings, and there are a few places where I cannot store the toml::key because the values are constructed artificially and only later written to a file.

[vaartis]

Well, answering my own question.. toml::table::find(key)->first seems to be the answer. Thank you again for implementing this, I'm hoping to finally get rid of all the ugly hacks I had.

[vaartis]

Actually, I think I found a bug.. Not with the keys in particular but with tables, they report their beginning on one symbol later then they actually are, i.e. if it starts at (line,column) 1,1 of the file, it reports it as 1,2. That means not on the [ of the first key, but on the first symbol of the key after [. Just tested this in 2.5.0 and that was not the case.

[marzer]

Oh, interesting. Mind opening a new issue with a small repro?

[vaartis]

#152 👍