Options style - #34
Merged
Merged
Conversation
…testing
Every option type took a pointer and returned nothing, so the options struct
escaped to the heap and each package read a little differently from the next.
They now chain by value, as go-openapi/core does:
type (
Option func(options) options
options struct{ ... }
)
func WithX(v T) Option {
return func(o options) options {
o.x = v
return o
}
}
applyWithDefaults folds the chain left to right. Where a package declares more
than one option type, the helper carries it: applyModWithDefaults,
applyTidyWithDefaults. Those two keep returning an error, which comes from
validating the assembled options rather than from any single option.
A default that is not a zero value stays seeded inside the helper. Nothing
declares an empty defaultOptions to match a package that has real defaults.
WithResolvedImports and WithBuildFlags write into a map and a slice the copy
shares, which stays correct because each chain starts from its own zero value.
An option that appended to a seeded slice would not: two chains would write
into one backing array, and into the seed itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…call
gofumpt's Extra.Set clears itself before reading a rule list, and
WithExtraRules called it once per name, so only the last one survived:
WithExtraRules("group_params", "clothe_returns") turned group_params back off.
Pass the names to Set the way gofumpt's -extra flag takes them, comma
separated, in a single call.
The tests only ever named one rule, which is why this held. The new one names
two and asserts the first survives the second.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
WithExtraRules rejects a rule gofumpt does not know, and a chain that passes
values has nowhere to put that error. It goes in an options struct of our own,
wrapping fumpt.Options beside the first failure:
type options struct {
fumpt fumpt.Options
err error
}
withError keeps the first failure and lets the rest of the chain run, and
Configure reads it before touching the package settings, so a failing option
still leaves the previous rules in place.
fumpt.Options holds no pointer - Extra is three bools - so the value the chain
carries is a plain copy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Option, SourceOption and DumpOption took a pointer and returned an error, so
the chain stopped at the first failure and the options struct escaped to the
heap. They now pass values, and the failure travels in the struct:
func (o options) withError(err error) options {
if o.err == nil {
o.err = err
}
return o
}
WithExtensions, WithRoots, WithExtraRoots, WithCoverage, Rebased and the four
From* constructors record their first bad argument there. New, Clone and Dump
read it before using the settings, so a caller sees the same error from the
same call as before. The options after a failing one now run, and none of them
touches anything outside the options value.
The defaults stay inside makeOptions and applyDumpWithDefaults rather than
moving to a package-level value: they hold a FuncMap and a slice, and building
them afresh per call is what keeps two repositories from sharing either.
For the same reason WithExtraRoots and the From* constructors clone the slice
they extend instead of appending in place, which would write into the backing
array of whichever chain got there first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Option in the repository package states where a rejected argument surfaces, and DumpOption did not. It behaves the same way: the option records the failure and Dump returns it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #34 +/- ##
==========================================
- Coverage 79.69% 79.69% -0.01%
==========================================
Files 79 79
Lines 4798 4861 +63
==========================================
+ Hits 3824 3874 +50
- Misses 973 986 +13
Partials 1 1 ☔ View full report in Codecov by Harness. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change type
Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update
Short description
Fixes
Full description
Checklist