Skip to content

Commit 58879ef

Browse files
jszobodyclaudehappy-otter
committed
Major shell refactoring: Centralize command configuration and remove duplication
- Created CommandRegistry as single source of truth for all command aliases and metadata - Created ArgumentProcessor to replace massive 75-line switch statement - Removed duplicated COMMAND_ALIASES from 4 different files - Removed dead code (addOptions, redundant resolveCommand) - Made interactive command handling extensible instead of special-cased for export - Cleaner separation of concerns throughout shell components All tests pass. Code is now much more maintainable and follows Laravel best practices. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
1 parent c1957f4 commit 58879ef

6 files changed

Lines changed: 308 additions & 184 deletions

File tree

‎src/Shell/ArgumentProcessor.php‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
<?php
2+
3+
namespace STS\Keep\Shell;
4+
5+
/**
6+
* Processes positional arguments for shell commands.
7+
* Replaces the massive switch statement with a clean, configurable approach.
8+
*/
9+
class ArgumentProcessor
10+
{
11+
/**
12+
* Command argument configurations
13+
* Defines how positional arguments map to command inputs
14+
*/
15+
private const CONFIGURATIONS = [
16+
'set' => [
17+
'arguments' => ['key', 'value'],
18+
],
19+
'get' => [
20+
'arguments' => ['key'],
21+
],
22+
'history' => [
23+
'arguments' => ['key'],
24+
],
25+
'delete' => [
26+
'arguments' => ['key'],
27+
'flags' => ['force'],
28+
],
29+
'copy' => [
30+
'arguments' => ['key'],
31+
'options' => [
32+
1 => '--to', // Second positional becomes --to option
33+
],
34+
],
35+
'import' => [
36+
'arguments' => ['from'],
37+
],
38+
'rename' => [
39+
'arguments' => ['old', 'new'],
40+
'flags' => ['force'],
41+
],
42+
'search' => [
43+
'arguments' => ['query'],
44+
'flags' => ['unmask', 'case-sensitive'],
45+
],
46+
'show' => [
47+
'flags' => ['unmask'],
48+
],
49+
'stage:add' => [
50+
'arguments' => ['name'],
51+
],
52+
];
53+
54+
/**
55+
* Process positional arguments for a command
56+
*/
57+
public static function process(string $command, array $positionals, array &$input): void
58+
{
59+
$config = self::CONFIGURATIONS[$command] ?? null;
60+
61+
if (!$config) {
62+
return;
63+
}
64+
65+
// Process regular arguments
66+
if (isset($config['arguments'])) {
67+
self::processArguments($positionals, $config['arguments'], $input);
68+
}
69+
70+
// Process flags (keywords that become boolean options)
71+
if (isset($config['flags'])) {
72+
self::processFlags($positionals, $config['flags'], $input);
73+
}
74+
75+
// Process options (positionals that map to specific options)
76+
if (isset($config['options'])) {
77+
self::processOptions($positionals, $config['options'], $input);
78+
}
79+
}
80+
81+
/**
82+
* Map positional arguments to named arguments
83+
*/
84+
private static function processArguments(array $positionals, array $argumentNames, array &$input): void
85+
{
86+
foreach ($argumentNames as $index => $name) {
87+
if (isset($positionals[$index])) {
88+
$input[$name] = $positionals[$index];
89+
}
90+
}
91+
}
92+
93+
/**
94+
* Convert flag keywords to boolean options
95+
*/
96+
private static function processFlags(array $positionals, array $flags, array &$input): void
97+
{
98+
foreach ($flags as $flag) {
99+
if (in_array($flag, $positionals)) {
100+
$input['--' . $flag] = true;
101+
}
102+
}
103+
}
104+
105+
/**
106+
* Map specific positional indices to options
107+
*/
108+
private static function processOptions(array $positionals, array $options, array &$input): void
109+
{
110+
foreach ($options as $index => $option) {
111+
if (isset($positionals[$index])) {
112+
$input[$option] = $positionals[$index];
113+
}
114+
}
115+
}
116+
}

‎src/Shell/CommandExecutor.php‎

Lines changed: 22 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,6 @@
99

1010
class CommandExecutor
1111
{
12-
private const COMMAND_ALIASES = [
13-
'g' => 'get',
14-
's' => 'set',
15-
'd' => 'delete',
16-
'l' => 'show',
17-
'ls' => 'show',
18-
];
19-
20-
private const NO_STAGE_COMMANDS = ['diff', 'copy', 'info', 'verify'];
21-
private const NO_VAULT_COMMANDS = ['export', 'copy', 'info', 'verify'];
22-
private const WRITE_COMMANDS = ['set', 'delete', 'copy', 'import', 'rename'];
2312

2413
public function __construct(
2514
private ShellContext $context,
@@ -39,16 +28,16 @@ public function execute(string $input): int
3928
$parsed = $this->parseInput($input);
4029
$commandName = $parsed['command'];
4130

42-
// Handle export command specially with interactive flow
43-
if ($commandName === 'export') {
44-
return $this->runInteractiveExport($parsed['positionals']);
31+
// Handle interactive commands (like export) with special flow
32+
if (CommandRegistry::isInteractive($commandName)) {
33+
return $this->runInteractiveCommand($commandName, $parsed['positionals']);
4534
}
4635

4736
$this->validateCommand($commandName);
4837

4938
$exitCode = $this->runCommand($commandName, $parsed);
5039

51-
if ($this->isWriteCommand($parsed['command'])) {
40+
if (CommandRegistry::isWriteCommand($parsed['command'])) {
5241
$this->context->invalidateCache();
5342
}
5443

@@ -64,24 +53,12 @@ protected function parseInput(string $input): array
6453
$parts = str_getcsv($input, ' ', '"', '\\');
6554
$command = array_shift($parts);
6655

67-
// In the interactive shell, we treat everything as positional arguments
68-
// No -- prefix support for better UX
69-
$positionals = $parts;
70-
$options = [];
71-
7256
return [
73-
'command' => $this->mapAlias($command),
74-
'positionals' => $positionals,
75-
'options' => $options,
57+
'command' => CommandRegistry::resolveAlias($command),
58+
'positionals' => $parts,
7659
];
7760
}
7861

79-
80-
protected function mapAlias(string $command): string
81-
{
82-
return self::COMMAND_ALIASES[$command] ?? $command;
83-
}
84-
8562
protected function validateCommand(string $commandName): void
8663
{
8764
if (!$this->application->has($commandName)) {
@@ -108,126 +85,28 @@ protected function buildCommandInput(string $commandName, array $parsed): array
10885
{
10986
$input = ['command' => $commandName];
11087

111-
// Process positionals and options based on command
112-
$this->processPositionalArguments(
113-
$parsed['command'],
114-
$parsed['positionals'],
115-
$input
116-
);
88+
// Use the new ArgumentProcessor for clean argument handling
89+
ArgumentProcessor::process($parsed['command'], $parsed['positionals'], $input);
11790

118-
$this->addOptions($parsed['options'], $input);
119-
120-
$this->addContextIfNeeded($parsed['command'], $parsed['options'], $input);
91+
// Add context (vault/stage) if needed
92+
$this->addContextIfNeeded($parsed['command'], $input);
12193

12294
return $input;
12395
}
12496

125-
protected function processPositionalArguments(string $command, array $positionals, array &$input): void
126-
{
127-
switch ($command) {
128-
case 'set':
129-
$this->addIfExists($positionals, 0, 'key', $input);
130-
$this->addIfExists($positionals, 1, 'value', $input);
131-
break;
132-
133-
case 'get':
134-
case 'history':
135-
// Just take the key, ignore any format options (shell is for humans)
136-
$this->addIfExists($positionals, 0, 'key', $input);
137-
break;
138-
139-
case 'export':
140-
// Export is handled by InteractiveExport, this shouldn't be reached
141-
break;
142-
143-
case 'copy':
144-
$this->addIfExists($positionals, 0, 'key', $input);
145-
// Handle optional destination argument
146-
if (isset($positionals[1])) {
147-
$input['--to'] = $positionals[1];
148-
}
149-
break;
150-
151-
case 'import':
152-
$this->addIfExists($positionals, 0, 'file', $input);
153-
break;
154-
155-
case 'stage:add':
156-
$this->addIfExists($positionals, 0, 'name', $input);
157-
break;
158-
159-
case 'show':
160-
// Only support 'unmask' in interactive shell (no format options)
161-
if (in_array('unmask', $positionals)) {
162-
$input['--unmask'] = true;
163-
}
164-
// Don't pass any positionals to show command
165-
$positionals = [];
166-
break;
167-
168-
case 'delete':
169-
// Handle "delete KEY force" without -- prefix
170-
foreach ($positionals as $index => $arg) {
171-
if ($arg === 'force' && $index > 0) {
172-
$input['--force'] = true;
173-
unset($positionals[$index]);
174-
}
175-
}
176-
// First positional is the key
177-
$this->addIfExists(array_values($positionals), 0, 'key', $input);
178-
break;
179-
180-
case 'rename':
181-
// Handle "rename OLD NEW [force]" without -- prefix
182-
$this->addIfExists($positionals, 0, 'old', $input);
183-
$this->addIfExists($positionals, 1, 'new', $input);
184-
if (in_array('force', $positionals)) {
185-
$input['--force'] = true;
186-
}
187-
break;
188-
189-
case 'search':
190-
// Handle "search QUERY [unmask] [case-sensitive]" without -- prefix
191-
$this->addIfExists($positionals, 0, 'query', $input);
192-
if (in_array('unmask', $positionals)) {
193-
$input['--unmask'] = true;
194-
}
195-
if (in_array('case-sensitive', $positionals)) {
196-
$input['--case-sensitive'] = true;
197-
}
198-
break;
199-
}
200-
}
201-
202-
protected function addIfExists(array $array, int $index, string $key, array &$target): void
203-
{
204-
if (isset($array[$index])) {
205-
$target[$key] = $array[$index];
206-
}
207-
}
20897

209-
protected function addOptions(array $options, array &$input): void
98+
protected function addContextIfNeeded(string $command, array &$input): void
21099
{
211-
foreach ($options as $key => $value) {
212-
$input['--' . $key] = $value;
213-
}
214-
}
215-
216-
protected function addContextIfNeeded(string $command, array $options, array &$input): void
217-
{
218-
$needsStage = !in_array($command, self::NO_STAGE_COMMANDS);
219-
$needsVault = !in_array($command, self::NO_VAULT_COMMANDS);
220-
221-
if ($needsStage && !isset($options['stage'])) {
100+
if (CommandRegistry::requiresStage($command) && !isset($input['--stage'])) {
222101
$input['--stage'] = $this->context->getStage();
223102
}
224103

225-
if ($needsVault && !isset($options['vault'])) {
104+
if (CommandRegistry::requiresVault($command) && !isset($input['--vault'])) {
226105
$input['--vault'] = $this->context->getVault();
227106
}
228107

229-
// Special handling for copy command
230-
if ($command === 'copy' && !isset($options['from'])) {
108+
// Special handling for copy command's 'from' context
109+
if ($command === 'copy' && !isset($input['--from'])) {
231110
$input['--from'] = sprintf(
232111
"%s:%s",
233112
$this->context->getVault(),
@@ -236,23 +115,21 @@ protected function addContextIfNeeded(string $command, array $options, array &$i
236115
}
237116
}
238117

239-
protected function isWriteCommand(string $command): bool
240-
{
241-
return in_array($command, self::WRITE_COMMANDS);
242-
}
243-
244118
protected function outputError(string $message): void
245119
{
246120
$output = new ConsoleOutput();
247121
$output->writeln("<error>{$message}</error>");
248122
}
249123

250124
/**
251-
* Run the interactive export command
125+
* Run an interactive command (like export)
252126
*/
253-
protected function runInteractiveExport(array $args): int
127+
protected function runInteractiveCommand(string $command, array $args): int
254128
{
255-
$interactiveExport = new InteractiveExport($this->context, $this->application);
256-
return $interactiveExport->execute($args);
129+
// Currently only export is interactive, but this is extensible
130+
return match ($command) {
131+
'export' => (new InteractiveExport($this->context, $this->application))->execute($args),
132+
default => throw new \Exception("Unknown interactive command: {$command}")
133+
};
257134
}
258135
}

0 commit comments

Comments
 (0)