go-toml icon indicating copy to clipboard operation
go-toml copied to clipboard

Bring back toml.Unmarshaler

Open pelletier opened this issue 1 year ago • 2 comments

Creating a main issue to track this work, as it has been asked multiple times, is probably hindering the adoption of go-toml v2.

The main difficulty of building this feature is modifying the unmarshaler in a way that allows it to retrieve actual bytes from the parser for the current target element without impacting performance.

The good thing is that solving this problem will also make implementingtoml.RawMessage pretty easy (https://github.com/pelletier/go-toml/issues/796)!

In my opinion, the work should probably be carried out in this order:

  1. Add the ability to retrieve the offset in the TOML document where a node's data starts. The recent merge of https://github.com/pelletier/go-toml/pull/860 introducing Position should help with that.
  2. Make the Decoder detect that its current target implements toml.Unmarshaler, then keep calling the parser until it finishes its current structure.
  3. Record the first node's start offset and the last node's end offset to retrieve the raw data from the input document.
  4. Feed the raw data to the UnmarshalTOML method.

Part 2 is probably the most difficult to pull off; the unmarshaling and parsing code is somewhat intertwined – though thanks to the intermediate partial AST design, separating both should be doable. The hard part is keeping the same performance level when decoding non-Unmarshaler targets and trying not to fully duplicate the Decoder (otherwise, it's twice the maintenance work :)).

Attention should also be given to detecting that the type implements toml.Unmarshaler. There's likely going to be a performance penalty there until some parser generation is implemented (https://github.com/pelletier/go-toml/pull/669, https://github.com/pelletier/go-toml/pull/758), but we should aim to minimize the impact of that check.

pelletier avatar May 19 '23 15:05 pelletier

#940 added only partial support, this shouldnt be close probably

rszyma avatar Mar 19 '24 22:03 rszyma

Thanks for catching it, didn't mean to close this issue!

pelletier avatar Mar 20 '24 00:03 pelletier