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

JSON, TOML, YAML: support for nested config data #112

Merged
merged 9 commits into from Jul 20, 2023
Merged

Conversation

peterbourgon
Copy link
Owner

@peterbourgon peterbourgon commented Jul 18, 2023

This PR is a simple adaptation of the TraverseMap function from jolheiser#1 (thanks, @jolheiser!) which is applied to the JSON, TOML, and YAML config file parsers more or less in the same way. It allows nested config data in any of those formats to map to flag names in a reasonably well-defined way.

Addresses #107 and #108. I'm pretty sure this isn't a breaking change, but I'd appreciate a review from anyone who'd care to give it a look.

edit: I broke fftoml for sure, I'll fix that.

This is a precursor to a much larger refactor, including many highly requested features, coming soon to theaters near you.

@@ -62,3 +67,19 @@ func WithTableDelimiter(d string) Option {
c.delimiter = d
}
}

// ParseError wraps all errors originating from the TOML parser.
type ParseError struct {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you decide to introduce a breaking change at some point I would suggest dropping the type ParseError struct for something like:

var ParseError = errors.New("error parsing TOML config")

// ...

func (c ConfigFileParser) Parse(...) {
    // ...
    if err := toml.NewDecoder(r).Decode(&m); err != nil {
        return fmt.Errorf("%w: %w", ParseError, err)
    }
    // ...

You would also need to upgrade to go1.20 for this change.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a mistake to introduce error types to this package. The next version will absolutely remove them.

@peterbourgon
Copy link
Owner Author

Thanks for your review.

@peterbourgon peterbourgon merged commit e267c41 into main Jul 20, 2023
3 checks passed
@peterbourgon peterbourgon deleted the traverse-map branch July 20, 2023 14:31
@jolheiser jolheiser mentioned this pull request Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants