Skip to content

doc: DecodeHook's stated default is the hook the code replaced - #2165

Open
ump45nose wants to merge 1 commit into
spf13:masterfrom
ump45nose:doc-decodehook-default-weak-slice
Open

ump45nose wants to merge 1 commit into
spf13:masterfrom
ump45nose:doc-decodehook-default-weak-slice

Conversation

@ump45nose

Copy link
Copy Markdown

Overview

DecodeHook's doc comment states the wrong default decode hook. One-line doc fix.

What problem does it solve?

viper.go:58-64 documents the default as:

//	 mapstructure.ComposeDecodeHookFunc(
//			mapstructure.StringToTimeDurationHookFunc(),
//			mapstructure.StringToSliceHookFunc(","),
//		)

but defaultDecoderConfig — the function that actually establishes the default, at viper.go:1003-1007 — uses Viper's own weak-slice hook, and the mapstructure one is explicitly commented out right above it:

decodeHook = mapstructure.ComposeDecodeHookFunc(
	mapstructure.StringToTimeDurationHookFunc(),
	// mapstructure.StringToSliceHookFunc(","),
	stringToWeakSliceHookFunc(","),
)

stringToWeakSliceHookFunc is described at viper.go:1026-1028: mapstructure v2.0.0's StringToSliceHookFunc checks that the target is a string slice, and this helper removes that check (viper.go:1035 only tests t.Kind() != reflect.Slice), so comma-splitting also reaches non-string slice fields, whose elements are then converted by WeaklyTypedInput. That is a real behavioural difference, not a naming nit — and this text is what shows up on pkg.go.dev for DecodeHook.

Change

Point the documented example at the hook that is actually used:

//	 mapstructure.ComposeDecodeHookFunc(
//			mapstructure.StringToTimeDurationHookFunc(),
//			stringToWeakSliceHookFunc(","),
//		)

The other three mentions of StringToSliceHookFunc in the repo are left alone because they are correct in place: viper.go:1005 is the commented-out line being replaced, viper.go:1026 is the comment explaining why it was replaced, and viper_test.go:900,938 pass the strict hook explicitly to test the override path.

Tests

Comment-only, so no test change is needed and no behaviour moves. I did not run go build / go test — the command layer in this session refused all build and test invocations, so I am stating that rather than implying CI ran locally. The change is inside a // doc comment above func DecodeHook, which cannot affect compilation; gofmt-relevant whitespace was preserved exactly (the tab-indented //\t block is untouched apart from the identifier).

Related issues

None. Checked the open PR list before starting: #2143 ("Export DefaultDecodeHook for composition") touches this same default in a different way — it extracts defaultDecoderConfig's chain into an exported DefaultDecodeHook() and does not edit lines 58-64 — so the two do not overlap textually, and this correction is worth landing either way. If #2143 merges first, the doc here should probably be reworded to name DefaultDecodeHook() instead of the inlined chain.

Note: this PR was prepared and submitted by an autonomous agent on behalf of ump45nose (Viper has no AI-disclosure rule, but the authorship seems worth stating plainly for a doc change).

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.

1 participant