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

Comments support in unstable/Parser #860

Merged
merged 5 commits into from
May 19, 2023
Merged

Comments support in unstable/Parser #860

merged 5 commits into from
May 19, 2023

Conversation

pelletier
Copy link
Owner

@pelletier pelletier commented Mar 28, 2023

This is the first part of completing issue #822. In this pull request, I have added functionality to the unstable/Parser to emit Comment nodes when instructed to do so using the KeepComments: true option.

I have also added a new method, unstable/Parser.Shape(), which provides the location (byte offset, column, line) of the beginning and end of each Node. Although some nodes are currently missing their location, I plan to fix that soon. This method should help retrieving whitespace when needed.

To demonstrate the use of the new KeepComments option, I have included the parser_test.go/ExampleParser_comments example. This function shows how to interpret comments in various positions within a document. Comments are emitted either as root expressions (when they are alone on their line) or as siblings of the element that precedes them. Within an array, groups of comments are represented as a single comment with multiple child comments.

I purposely did not attach comments to any specific node, as I feel this is subject to interpretation and may vary from one application to another. Instead, the caller is free to decide which element a comment should be logically attached to, if at all.

@mikefarah @TomWright I welcome any feedback on this draft! Note that this pull request does not address how to encode the AST to TOML, but I plan to address that in a follow-up PR.


goos: linux
goarch: amd64
pkg: github.com/pelletier/go-toml/v2/benchmark
cpu: AMD Ryzen 9 5950X 16-Core Processor
                                   │  init2.txt  │            comments2.txt            │
                                   │   sec/op    │    sec/op     vs base               │
UnmarshalDataset/config-16           16.31m ± 0%    16.91m ± 0%  +3.67% (p=0.000 n=10)
UnmarshalDataset/canada-16           59.50m ± 1%    59.90m ± 2%       ~ (p=0.579 n=10)
UnmarshalDataset/citm_catalog-16     19.90m ± 1%    19.91m ± 2%       ~ (p=0.631 n=10)
UnmarshalDataset/twitter-16          8.201m ± 2%    8.231m ± 1%       ~ (p=0.315 n=10)
UnmarshalDataset/code-16             73.05m ± 0%    75.04m ± 0%  +2.72% (p=0.000 n=10)
UnmarshalDataset/example-16          127.0µ ± 0%    128.6µ ± 0%  +1.23% (p=0.000 n=10)
Unmarshal/SimpleDocument/struct-16   461.6n ± 0%    481.1n ± 0%  +4.21% (p=0.000 n=10)
Unmarshal/SimpleDocument/map-16      643.4n ± 1%    663.6n ± 0%  +3.15% (p=0.000 n=10)
Unmarshal/ReferenceFile/struct-16    36.79µ ± 0%    37.15µ ± 0%  +0.97% (p=0.000 n=10)
Unmarshal/ReferenceFile/map-16       56.81µ ± 0%    56.95µ ± 0%       ~ (p=0.143 n=10)
Unmarshal/HugoFrontMatter-16         9.962µ ± 0%   10.073µ ± 0%  +1.11% (p=0.000 n=10)
geomean                              348.1µ         353.9µ       +1.66%

@pelletier pelletier added the feature Issue asking for a new feature in go-toml. label Mar 28, 2023
@mikefarah
Copy link

Great! Will give it a go when I get some time :)

@pelletier pelletier marked this pull request as ready for review May 19, 2023 14:41
@pelletier
Copy link
Owner Author

I'm going to merge this as it's only in unstable, and I suspect may be lightly conflicting with @ImSingee's work on toml.Unmarshaler.

@pelletier pelletier merged commit fcf9d37 into v2 May 19, 2023
9 checks passed
@pelletier pelletier deleted the comments branch May 19, 2023 14:44
@hhildebrandt
Copy link

Interesting extension! Can you give an outlook when the comment support will be officially tagged?
BTW - The current version is not buildable, because in go.mod go version 1.16 is pulled, but here a function from 1.17 is used: https://github.com/pelletier/go-toml/blob/44c1513ccd8d946f2be4e1fafd5675be00de0d6f/unmarshaler.go#LL1030C7-L1030C24

@pelletier
Copy link
Owner Author

@hhildebrandt released :) https://github.com/pelletier/go-toml/releases/tag/v2.0.8 and thank you for the heads up about PointerTo.

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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants