Skip to content

Commit 3ee12c4

Browse files
Do not export empty, disabled, or included configs
This change is done to not clutter the configuration with unnecessary hook configurations, and to make sure when changing a configuration file with 'enable' or 'disable' commands, we do not double actions that are included from other files. Issue: #278
1 parent a563a35 commit 3ee12c4

File tree

6 files changed

+64
-9
lines changed

6 files changed

+64
-9
lines changed

src/Config.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,9 @@ public function getJsonData(): array
412412
$data['config']['custom'] = $this->custom;
413413
}
414414
foreach (Hooks::getValidHooks() as $hook => $value) {
415-
$data[$hook] = $this->hooks[$hook]->getJsonData();
415+
if ($this->hooks[$hook]->isEnabled() || $this->hooks[$hook]->hasActions()) {
416+
$data[$hook] = $this->hooks[$hook]->getJsonData();
417+
}
416418
}
417419
return $data;
418420
}

src/Config/Action.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@ class Action
6161
Config::SETTING_LABEL
6262
];
6363

64+
/**
65+
* Indicates if an action config was included from another file
66+
*
67+
* @var bool
68+
*/
69+
private bool $isIncluded = false;
70+
6471
/**
6572
* Action constructor
6673
*
@@ -114,6 +121,26 @@ private function setupSettings(array $settings): void
114121
}
115122
}
116123

124+
/**
125+
* Marks a action config as included
126+
*
127+
* @return void
128+
*/
129+
public function markIncluded(): void
130+
{
131+
$this->isIncluded = true;
132+
}
133+
134+
/**
135+
* Check if an action config was included
136+
*
137+
* @return bool
138+
*/
139+
public function isIncluded(): bool
140+
{
141+
return $this->isIncluded;
142+
}
143+
117144
/**
118145
* Indicates if the action can fail without stopping the git operation
119146
*

src/Config/Factory.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ private function mergeHookConfigFromIncludes(Hook $hook, array $includes): void
291291
// empty hook config just to enable the included actions feels a bit dull.
292292
// Since the main hook is processed last (if one is configured) the enabled flag will be overwritten
293293
// once again by the main config value. This is to make sure that if somebody disables a hook in its
294-
// main configuration no actions will get executed, even if we have enabled hooks in any include file.
294+
// main configuration, no actions will get executed, even if we have enabled hooks in any include file.
295295
$this->copyActionsFromTo($includedHook, $hook);
296296
}
297297
}
@@ -326,6 +326,7 @@ protected function loadIncludedConfigs(array $json, string $path): array
326326
private function copyActionsFromTo(Hook $sourceConfig, Hook $targetConfig): void
327327
{
328328
foreach ($sourceConfig->getActions() as $action) {
329+
$action->markIncluded();
329330
$targetConfig->addAction($action);
330331
}
331332
}

src/Config/Hook.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,16 @@ public function isEnabled(): bool
8686
return $this->isEnabled;
8787
}
8888

89+
/**
90+
* Check if a hook config has actions
91+
*
92+
* @return bool
93+
*/
94+
public function hasActions(): bool
95+
{
96+
return !empty($this->actions);
97+
}
98+
8999
/**
90100
* Add an action to the list
91101
*
@@ -118,7 +128,9 @@ public function getJsonData(): array
118128
{
119129
$config = ['enabled' => $this->isEnabled, 'actions' => []];
120130
foreach ($this->actions as $action) {
121-
$config['actions'][] = $action->getJsonData();
131+
if (!$action->isIncluded()) {
132+
$config['actions'][] = $action->getJsonData();
133+
}
122134
}
123135
return $config;
124136
}

tests/unit/ConfigTest.php

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,15 +157,17 @@ public function testFailOnFirstErrorCanBeChanged(): void
157157
$this->assertFalse($config->failOnFirstError());
158158
}
159159

160-
public function testCanBeExportedToJsonData(): void
160+
public function testExportsOnlyEnabledHooksOrHooksWithActionsToJsonData(): void
161161
{
162162
$config = new Config('./no-config.json');
163+
$config->getHookConfig('pre-commit')->setEnabled(true);
164+
$config->getHookConfig('pre-push')->addAction(new Action('foo'));
163165
$json = $config->getJsonData();
164166

165167
$this->assertIsArray($json);
166168
$this->assertIsArray($json['pre-commit']);
167-
$this->assertIsArray($json['commit-msg']);
168169
$this->assertIsArray($json['pre-push']);
170+
$this->assertFalse(array_key_exists('commit-msg', $json));
169171
}
170172

171173
public function testCanBeExportedToJsonDataWithSettings(): void
@@ -180,9 +182,6 @@ public function testCanBeExportedToJsonDataWithSettings(): void
180182
$this->assertIsArray($json);
181183
$this->assertIsArray($json['config']);
182184
$this->assertIsArray($json['config']['run']);
183-
$this->assertIsArray($json['pre-commit']);
184-
$this->assertIsArray($json['commit-msg']);
185-
$this->assertIsArray($json['pre-push']);
186185
}
187186

188187
public function testGetJsonDataWithoutEmptyConfig(): void
@@ -204,6 +203,21 @@ public function testGetJsonDataWithConfigSection(): void
204203
$this->assertArrayNotHasKey('plugins', $json);
205204
}
206205

206+
public function testDoesNotExportIncludedActionsToJson(): void
207+
{
208+
$localAction = new Action('foo');
209+
$includedAction = new Action('bar');
210+
$includedAction->markIncluded();
211+
212+
$config = new Config('foo.json', true, []);
213+
$config->getHookConfig('pre-commit')->setEnabled(true);
214+
$config->getHookConfig('pre-commit')->addAction($localAction);
215+
$config->getHookConfig('pre-commit')->addAction($includedAction);
216+
217+
$json = $config->getJsonData();
218+
$this->assertCount(1, $json['pre-commit']['actions']);
219+
}
220+
207221
public function testGetPluginsReturnsEmptyArray(): void
208222
{
209223
$config = new Config('foo.json');

tests/unit/Runner/Config/CreatorTest.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ public function testConfigureFileExtend(): void
7676
$configFileContentAfter = $configDir->getChild('captainhook.json')->getContent();
7777
$this->assertFileExists($configFile);
7878
$this->assertStringContainsString('pre-commit', $configFileContentAfter);
79-
$this->assertStringContainsString('pre-push', $configFileContentAfter);
8079
$this->assertStringContainsString('phpunit', $configFileContentAfter);
8180
}
8281
}

0 commit comments

Comments
 (0)