Skip to content

refactor: collapse opts merging to single authoritative path#13

Merged
zuqini merged 1 commit intomainfrom
zuqini/opt-merge-refactor
Apr 11, 2026
Merged

refactor: collapse opts merging to single authoritative path#13
zuqini merged 1 commit intomainfrom
zuqini/opt-merge-refactor

Conversation

@zuqini
Copy link
Copy Markdown
Owner

@zuqini zuqini commented Apr 11, 2026

Remove the DEEP_MERGE strategy and drop opts from merge_specs entirely. Function-form opts could never be resolved at merge time (the plugin arg doesn't exist yet), so resolve_opts already had to run at load time — the merge-time DEEP_MERGE pass just produced an orphaned value that no runtime code read for its value, only for its nil-ness. This left two parallel opts pipelines and made merged_spec.opts subtly wrong whenever function-form and table-form were mixed.

Now:

  • merge_specs skips opts. merged_spec.opts is always nil.
  • resolve_all computes entry.has_opts once, a boolean summary used by the three existence checks in plugin_loader and startup.
  • run_config always calls resolve_opts(sorted_specs, plugin) — the single-spec fast path is gone, which has the side benefit of always handing config() a fresh table instead of a live reference to the spec's underlying opts.
  • merge_spec_array now always folds from an empty base, so the returned table is always freshly allocated and never aliases an input spec.

Tests updated to reflect the new contract:

  • opts_test.lua: assert merged_spec.opts == nil and check entry.has_opts / sorted_specs[1].opts instead.
  • merge_test.lua: the old "merge_specs deep merges opts" test now asserts merge_specs skips opts and that resolve_opts is the deep-merge path.
  • is_single_spec_test.lua: same treatment.

Remove the DEEP_MERGE strategy and drop `opts` from `merge_specs` entirely.
Function-form opts could never be resolved at merge time (the `plugin` arg
doesn't exist yet), so `resolve_opts` already had to run at load time — the
merge-time DEEP_MERGE pass just produced an orphaned value that no runtime
code read for its value, only for its nil-ness. This left two parallel opts
pipelines and made `merged_spec.opts` subtly wrong whenever function-form
and table-form were mixed.

Now:
- `merge_specs` skips `opts`. `merged_spec.opts` is always nil.
- `resolve_all` computes `entry.has_opts` once, a boolean summary used by
  the three existence checks in `plugin_loader` and `startup`.
- `run_config` always calls `resolve_opts(sorted_specs, plugin)` — the
  single-spec fast path is gone, which has the side benefit of always
  handing `config()` a fresh table instead of a live reference to the
  spec's underlying opts.
- `merge_spec_array` now always folds from an empty base, so the returned
  table is always freshly allocated and never aliases an input spec.

Tests updated to reflect the new contract:
- `opts_test.lua`: assert `merged_spec.opts == nil` and check
  `entry.has_opts` / `sorted_specs[1].opts` instead.
- `merge_test.lua`: the old "merge_specs deep merges opts" test now asserts
  merge_specs skips opts and that `resolve_opts` is the deep-merge path.
- `is_single_spec_test.lua`: same treatment.
@zuqini zuqini force-pushed the zuqini/opt-merge-refactor branch from f674275 to 8cc4bb2 Compare April 11, 2026 01:26
@zuqini zuqini merged commit d3cc74d into main Apr 11, 2026
2 checks passed
@zuqini zuqini deleted the zuqini/opt-merge-refactor branch April 11, 2026 01:34
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