Skip to content

Commit ae92886

Browse files
authored
feat: extend config merge with non-plugin field semantics (#254)
* feat: extend MergeConfig with non-plugin field merging (theme, extensions, hooks) Rename MergePluginConfig to MergeConfig (deprecated alias preserved). Theme: module wins if set. Extensions: additive merge deduplicated by name. PreReleaseHooks: additive merge (root first, then module). All slice fields are deep-copied to prevent mutation of root config. * test: MergeConfig non-plugin field merge semantics and deep copy safety * refactor: use maps.Copy for shallow map cloning in config merge helpers
1 parent ce3c1c2 commit ae92886

4 files changed

Lines changed: 300 additions & 7 deletions

File tree

internal/commands/bump/helpers.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func applyModuleTagPrefix(tm tagmanager.TagManager, bumpedPath string, cfg *conf
120120
if moduleCfg == nil {
121121
return noop, nil
122122
}
123-
mergedCfg := config.MergePluginConfig(cfg, moduleCfg)
123+
mergedCfg := config.MergeConfig(cfg, moduleCfg)
124124
if mergedCfg.Plugins == nil || mergedCfg.Plugins.TagManager == nil {
125125
return noop, nil
126126
}

internal/commands/tag/tagcmd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func resolveModuleConfig(cfg *config.Config, path string) (*config.Config, strin
147147
if err != nil || moduleCfg == nil {
148148
return cfg, moduleDir
149149
}
150-
return config.MergePluginConfig(cfg, moduleCfg), moduleDir
150+
return config.MergeConfig(cfg, moduleCfg), moduleDir
151151
}
152152

153153
// runCreateCmd creates a git tag for the current version.

internal/config/config.go

Lines changed: 116 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package config
33
import (
44
"bytes"
55
"fmt"
6+
"maps"
67
"os"
78
"path/filepath"
89
"strings"
@@ -202,11 +203,22 @@ func LoadConfigFromDir(dir string) (*Config, error) {
202203
return loadConfigFromPath(filepath.Join(dir, ".sley.yaml"))
203204
}
204205

205-
// MergePluginConfig merges a module-level config into a root config.
206+
// MergePluginConfig is a deprecated alias for [MergeConfig].
207+
var MergePluginConfig = MergeConfig
208+
209+
// MergeConfig merges a module-level config into a root config.
206210
// Plugin pointer fields from module override root when non-nil.
207211
// CommitParser always comes from root (workspace-level setting).
212+
//
213+
// Non-plugin field semantics:
214+
// - Path: always root
215+
// - Workspace: always root
216+
// - Theme: module wins if non-empty, else root
217+
// - Extensions: additive merge (root + module, dedup by Name, module wins)
218+
// - PreReleaseHooks: additive merge (root hooks then module hooks appended)
219+
//
208220
// Returns a new *Config without mutating the inputs.
209-
func MergePluginConfig(root, module *Config) *Config {
221+
func MergeConfig(root, module *Config) *Config {
210222
if root == nil && module == nil {
211223
return nil
212224
}
@@ -217,11 +229,17 @@ func MergePluginConfig(root, module *Config) *Config {
217229
return module
218230
}
219231

232+
// Theme: module wins if non-empty.
233+
theme := root.Theme
234+
if module.Theme != "" {
235+
theme = module.Theme
236+
}
237+
220238
merged := &Config{
221239
Path: root.Path,
222-
Theme: root.Theme,
223-
Extensions: root.Extensions,
224-
PreReleaseHooks: root.PreReleaseHooks,
240+
Theme: theme,
241+
Extensions: mergeExtensions(root.Extensions, module.Extensions),
242+
PreReleaseHooks: mergePreReleaseHooks(root.PreReleaseHooks, module.PreReleaseHooks),
225243
Workspace: root.Workspace,
226244
}
227245

@@ -280,3 +298,96 @@ func NormalizeVersionPath(path string) string {
280298
// ConfigFilePerm defines secure file permissions for config files (owner read/write only).
281299
// References core.PermOwnerRW for consistency across the codebase.
282300
const ConfigFilePerm = core.PermOwnerRW
301+
302+
// mergeExtensions performs an additive merge of root and module extension slices.
303+
// Deduplication is by Name: if the same Name appears in both, the module version wins.
304+
// Order: root extensions first (with duplicates replaced), then module-only extensions appended.
305+
// Returns a new slice; inputs are not modified.
306+
func mergeExtensions(root, module []ExtensionConfig) []ExtensionConfig {
307+
if len(module) == 0 {
308+
return copyExtensions(root)
309+
}
310+
if len(root) == 0 {
311+
return copyExtensions(module)
312+
}
313+
314+
// Build lookup of module extensions by Name.
315+
moduleByName := make(map[string]ExtensionConfig, len(module))
316+
for _, ext := range module {
317+
moduleByName[ext.Name] = ext
318+
}
319+
320+
seen := make(map[string]bool, len(root)+len(module))
321+
result := make([]ExtensionConfig, 0, len(root)+len(module))
322+
323+
// Start with root order; replace duplicates with module version.
324+
for _, ext := range root {
325+
if mExt, ok := moduleByName[ext.Name]; ok {
326+
result = append(result, copyExtension(mExt))
327+
} else {
328+
result = append(result, copyExtension(ext))
329+
}
330+
seen[ext.Name] = true
331+
}
332+
333+
// Append module-only extensions.
334+
for _, ext := range module {
335+
if !seen[ext.Name] {
336+
result = append(result, copyExtension(ext))
337+
}
338+
}
339+
340+
return result
341+
}
342+
343+
// mergePreReleaseHooks performs an additive merge: deep-copied root hooks followed
344+
// by deep-copied module hooks. Returns a new slice; inputs are not modified.
345+
func mergePreReleaseHooks(root, module []map[string]PreReleaseHookConfig) []map[string]PreReleaseHookConfig {
346+
total := len(root) + len(module)
347+
if total == 0 {
348+
return nil
349+
}
350+
351+
result := make([]map[string]PreReleaseHookConfig, 0, total)
352+
for _, m := range root {
353+
result = append(result, copyHookMap(m))
354+
}
355+
for _, m := range module {
356+
result = append(result, copyHookMap(m))
357+
}
358+
return result
359+
}
360+
361+
// copyExtensions returns a deep copy of the given extension slice.
362+
// Returns nil for nil or empty input.
363+
func copyExtensions(exts []ExtensionConfig) []ExtensionConfig {
364+
if len(exts) == 0 {
365+
return nil
366+
}
367+
out := make([]ExtensionConfig, len(exts))
368+
for i, ext := range exts {
369+
out[i] = copyExtension(ext)
370+
}
371+
return out
372+
}
373+
374+
// copyExtension returns a deep copy of a single ExtensionConfig,
375+
// including its Config map.
376+
func copyExtension(ext ExtensionConfig) ExtensionConfig {
377+
cp := ext
378+
if ext.Config != nil {
379+
cp.Config = make(map[string]any, len(ext.Config))
380+
maps.Copy(cp.Config, ext.Config)
381+
}
382+
return cp
383+
}
384+
385+
// copyHookMap returns a deep copy of a single pre-release hook map.
386+
func copyHookMap(m map[string]PreReleaseHookConfig) map[string]PreReleaseHookConfig {
387+
if m == nil {
388+
return nil
389+
}
390+
cp := make(map[string]PreReleaseHookConfig, len(m))
391+
maps.Copy(cp, m)
392+
return cp
393+
}

internal/config/config_test.go

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,188 @@ func TestLoadConfigFromDir(t *testing.T) {
105105
}
106106
}
107107

108+
func TestMergeConfig_Theme(t *testing.T) {
109+
t.Parallel()
110+
tests := []struct {
111+
name string
112+
rootTheme string
113+
modTheme string
114+
want string
115+
}{
116+
{"module_has_theme", "sley", "dracula", "dracula"},
117+
{"module_has_no_theme", "sley", "", "sley"},
118+
{"both_empty", "", "", ""},
119+
}
120+
for _, tt := range tests {
121+
t.Run(tt.name, func(t *testing.T) {
122+
t.Parallel()
123+
root := &Config{Theme: tt.rootTheme}
124+
mod := &Config{Theme: tt.modTheme}
125+
merged := MergeConfig(root, mod)
126+
if merged.Theme != tt.want {
127+
t.Errorf("Theme = %q, want %q", merged.Theme, tt.want)
128+
}
129+
})
130+
}
131+
}
132+
133+
func TestMergeConfig_Extensions_Additive(t *testing.T) {
134+
t.Parallel()
135+
tests := []struct {
136+
name string
137+
root []ExtensionConfig
138+
module []ExtensionConfig
139+
want []string // expected names in order
140+
wantNil bool
141+
}{
142+
{
143+
"root_A_module_B",
144+
[]ExtensionConfig{{Name: "docker-sync"}},
145+
[]ExtensionConfig{{Name: "cargo-check"}},
146+
[]string{"docker-sync", "cargo-check"},
147+
false,
148+
},
149+
{
150+
"duplicate_by_name_module_wins",
151+
[]ExtensionConfig{{Name: "docker-sync", Path: "a"}},
152+
[]ExtensionConfig{{Name: "docker-sync", Path: "b"}},
153+
[]string{"docker-sync"},
154+
false,
155+
},
156+
{
157+
"module_empty",
158+
[]ExtensionConfig{{Name: "docker-sync"}},
159+
nil,
160+
[]string{"docker-sync"},
161+
false,
162+
},
163+
{
164+
"root_empty",
165+
nil,
166+
[]ExtensionConfig{{Name: "cargo-check"}},
167+
[]string{"cargo-check"},
168+
false,
169+
},
170+
{
171+
"both_empty",
172+
nil,
173+
nil,
174+
nil,
175+
true,
176+
},
177+
}
178+
for _, tt := range tests {
179+
t.Run(tt.name, func(t *testing.T) {
180+
t.Parallel()
181+
root := &Config{Extensions: tt.root}
182+
mod := &Config{Extensions: tt.module}
183+
merged := MergeConfig(root, mod)
184+
if tt.wantNil {
185+
if merged.Extensions != nil {
186+
t.Errorf("expected nil extensions, got %v", merged.Extensions)
187+
}
188+
return
189+
}
190+
if len(merged.Extensions) != len(tt.want) {
191+
t.Fatalf("expected %d extensions, got %d: %v", len(tt.want), len(merged.Extensions), merged.Extensions)
192+
}
193+
for i, name := range tt.want {
194+
if merged.Extensions[i].Name != name {
195+
t.Errorf("ext[%d] = %q, want %q", i, merged.Extensions[i].Name, name)
196+
}
197+
}
198+
// Check duplicate: module path wins
199+
if tt.name == "duplicate_by_name_module_wins" {
200+
if merged.Extensions[0].Path != "b" {
201+
t.Errorf("duplicate: expected module path 'b', got %q", merged.Extensions[0].Path)
202+
}
203+
}
204+
})
205+
}
206+
}
207+
208+
func TestMergeConfig_PreReleaseHooks_Additive(t *testing.T) {
209+
t.Parallel()
210+
tests := []struct {
211+
name string
212+
root []map[string]PreReleaseHookConfig
213+
module []map[string]PreReleaseHookConfig
214+
want int // expected total hooks
215+
}{
216+
{"root_plus_module", []map[string]PreReleaseHookConfig{{"lint": {Command: "lint"}}}, []map[string]PreReleaseHookConfig{{"test": {Command: "test"}}}, 2},
217+
{"module_only", nil, []map[string]PreReleaseHookConfig{{"test": {Command: "test"}}}, 1},
218+
{"both_empty", nil, nil, 0},
219+
}
220+
for _, tt := range tests {
221+
t.Run(tt.name, func(t *testing.T) {
222+
t.Parallel()
223+
root := &Config{PreReleaseHooks: tt.root}
224+
mod := &Config{PreReleaseHooks: tt.module}
225+
merged := MergeConfig(root, mod)
226+
got := len(merged.PreReleaseHooks)
227+
if got != tt.want {
228+
t.Errorf("expected %d hooks, got %d", tt.want, got)
229+
}
230+
})
231+
}
232+
}
233+
234+
func TestMergeConfig_DeepCopy(t *testing.T) {
235+
t.Parallel()
236+
237+
t.Run("extensions_deep_copy", func(t *testing.T) {
238+
t.Parallel()
239+
root := &Config{Extensions: []ExtensionConfig{{Name: "original", Path: "/root"}}}
240+
mod := &Config{Extensions: []ExtensionConfig{{Name: "added"}}}
241+
merged := MergeConfig(root, mod)
242+
243+
// Mutate merged
244+
merged.Extensions[0].Name = "mutated"
245+
246+
// Root must be unchanged
247+
if root.Extensions[0].Name != "original" {
248+
t.Errorf("root was mutated: got %q, want 'original'", root.Extensions[0].Name)
249+
}
250+
})
251+
252+
t.Run("hooks_deep_copy", func(t *testing.T) {
253+
t.Parallel()
254+
root := &Config{PreReleaseHooks: []map[string]PreReleaseHookConfig{{"lint": {Command: "lint"}}}}
255+
mod := &Config{}
256+
merged := MergeConfig(root, mod)
257+
258+
// Mutate merged
259+
merged.PreReleaseHooks[0]["lint"] = PreReleaseHookConfig{Command: "mutated"}
260+
261+
// Root must be unchanged
262+
if root.PreReleaseHooks[0]["lint"].Command != "lint" {
263+
t.Errorf("root was mutated: got %q, want 'lint'", root.PreReleaseHooks[0]["lint"].Command)
264+
}
265+
})
266+
}
267+
268+
func TestMergeConfig_PathAlwaysRoot(t *testing.T) {
269+
t.Parallel()
270+
root := &Config{Path: ".version"}
271+
mod := &Config{Path: "custom.version"}
272+
merged := MergeConfig(root, mod)
273+
if merged.Path != ".version" {
274+
t.Errorf("Path = %q, want '.version' (root always)", merged.Path)
275+
}
276+
}
277+
278+
func TestMergeConfig_WorkspaceAlwaysRoot(t *testing.T) {
279+
t.Parallel()
280+
rootWs := &WorkspaceConfig{Modules: []ModuleConfig{{Name: "root-mod"}}}
281+
modWs := &WorkspaceConfig{Modules: []ModuleConfig{{Name: "module-mod"}}}
282+
root := &Config{Workspace: rootWs}
283+
mod := &Config{Workspace: modWs}
284+
merged := MergeConfig(root, mod)
285+
if merged.Workspace != rootWs {
286+
t.Error("expected workspace from root, not module")
287+
}
288+
}
289+
108290
func TestMergePluginConfig(t *testing.T) {
109291
t.Parallel()
110292

0 commit comments

Comments
 (0)