Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring back toml.Unmarshaler #873

Open
pelletier opened this issue May 19, 2023 · 2 comments · Fixed by #940
Open

Bring back toml.Unmarshaler #873

pelletier opened this issue May 19, 2023 · 2 comments · Fixed by #940
Labels
feature Issue asking for a new feature in go-toml.

Comments

@pelletier
Copy link
Owner

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 (#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 Comments support in unstable/Parser #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 (#669, #758), but we should aim to minimize the impact of that check.

@rszyma
Copy link
Contributor

rszyma commented Mar 19, 2024

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

@pelletier pelletier reopened this Mar 20, 2024
@pelletier
Copy link
Owner Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Issue asking for a new feature in go-toml.
Projects
None yet
2 participants