Skip to content

Refactor: all plugins use functional option pattern - #358

Draft
Loyalsoldier with Claude wants to merge 5 commits into
refactor-plugin-2from
claude/change-plugins-to-functional-options
Draft

Refactor: all plugins use functional option pattern#358
Loyalsoldier with Claude wants to merge 5 commits into
refactor-plugin-2from
claude/change-plugins-to-functional-options

Conversation

@Claude

@Claude Claude AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Extends the functional option pattern introduced for plugin/singbox (commit 182fdc0) to every converter under plugin/, so converters can be built programmatically with validated options instead of raw struct literals.

Constructor shape

Each converter now exposes:

  • New*(action, opts ...lib.InputOption) / (..., opts ...lib.OutputOption) — applies options, validates, returns the converter
  • New*FromBytes(action, data []byte) — unmarshals JSON config and delegates to New*
  • With* functions for each individual field

The concrete structs are unexported (snake_case: mrs_in, text_out, geolite2_asn_csv_in, cutter, …), so construction goes through the constructors.

input := plaintext.NewTextIn(
    plaintext.TypeTextIn,
    plaintext.DescTextIn,
    lib.ActionAdd,
    plaintext.WithNameAndURI(name, uri),
    plaintext.WithInputDir(dir),
)

Validation

Required/optional derived from configuration.md; invalid combinations abort via log.Fatalf in New*:

  • name and uri are a single WithNameAndURI(name, uri) option — specifying one without the other is rejected
  • inputDir required when neither name nor uri is given (clashRuleSet, clashRuleSetClassical, json, mihomoMRS, singboxSRS, surgeRuleSet, text)
  • v2rayGeoIPDat input requires uri; json requires jsonPath; stdin requires name; cutter requires remove action plus non-empty wantedList; lookup output requires search

Option naming

Packages with a single in/out converter (mihomo, v2ray, singbox) use the generic names. Packages with several converters prefix per converter to avoid collisions:

  • maxmind: WithMMDBIn*, WithASNCSV*, WithCountryCSV* for inputs; generic WithOutput* + WithSourceMMDBURI for the single output
  • special: WithCutter*, WithPrivateOnlyIPType, WithStdin*, WithStdout*, WithSearch/WithSearchList
  • plaintext: all input types share text_in and all output types share text_out, so generic names work with iType/iDesc passed to the constructor

Defaults

Type-dependent defaults (plaintext output dir/extension, maxmind default MMDB URI and CSV paths, maxmind output dir/name) are seeded in the constructor before options are applied; the matching With* is a no-op on empty input, preserving current behavior for configs that omit them.

Callers

Root lookup.go and merge.go now build converters via the new constructors rather than struct literals.

Claude AI and others added 2 commits July 29, 2026 13:44
@Loyalsoldier

Copy link
Copy Markdown
Owner

@copilot Review this PR, try to refine it and find potential bugs.

@Loyalsoldier

Copy link
Copy Markdown
Owner

@codex[agent] Review this PR, try to refine it and find potential bugs.

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

@copilot Review this PR, try to refine it and find potential bugs.

Reviewed and fixed the lookup --dir regression plus MRS/SRS source/default validation in 7563936; refinements are in a6d6e93 and 9c73aee. Full tests/build pass, and final review/CodeQL are clean.

@Codex

Codex AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

@codex[agent] Review this PR, try to refine it and find potential bugs.

Reviewed the PR at 9c73aee and rechecked the functional-option constructors, lookup source selection, and validation paths. I did not find additional regressions to patch; go test ./... and go build ./ both pass locally.

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.

4 participants