Skip to content

Commit 9edbb39

Browse files
paulbalandanpaulbalandan
committed
refactor: improve UX in logs:clear command
1 parent df9f137 commit 9edbb39

3 files changed

Lines changed: 44 additions & 30 deletions

File tree

‎system/Commands/Housekeeping/ClearLogs.php‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ protected function interact(array &$arguments, array &$options): void
3939
return;
4040
}
4141

42-
if (CLI::prompt('Are you sure you want to delete the logs?', ['n', 'y']) === 'n') {
42+
if (CLI::prompt(sprintf('Delete all log files in %s?', clean_path(WRITEPATH . 'logs')), ['n', 'y']) === 'n') {
4343
return;
4444
}
4545

@@ -49,24 +49,26 @@ protected function interact(array &$arguments, array &$options): void
4949
protected function execute(array $arguments, array $options): int
5050
{
5151
if ($options['force'] === false) {
52-
CLI::error('Deleting logs aborted.');
52+
if ($this->isInteractive()) {
53+
CLI::write('Log deletion cancelled.', 'yellow');
5354

54-
if (! $this->isInteractive()) {
55-
CLI::error('If you want, use the "--force" option to force delete all log files.');
55+
return EXIT_SUCCESS;
5656
}
5757

58+
CLI::error('Log deletion aborted: pass --force to delete log files in non-interactive mode.');
59+
5860
return EXIT_ERROR;
5961
}
6062

6163
helper('filesystem');
6264

6365
if (! delete_files(WRITEPATH . 'logs', htdocs: true)) {
64-
CLI::error('Error in deleting the logs files.');
66+
CLI::error(sprintf('Failed to delete log files in %s.', clean_path(WRITEPATH . 'logs')));
6567

6668
return EXIT_ERROR;
6769
}
6870

69-
CLI::write('Logs cleared.', 'green');
71+
CLI::write(sprintf('Log files in %s cleared.', clean_path(WRITEPATH . 'logs')), 'green');
7072

7173
return EXIT_SUCCESS;
7274
}

‎tests/system/Commands/Housekeeping/ClearLogsTest.php‎

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,13 @@ public function testClearLogsUsingForce(): void
8181

8282
$this->assertFileDoesNotExist(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log");
8383
$this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . 'index.html');
84-
$this->assertSame("\nLogs cleared.\n", preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer()));
84+
$this->assertSame(
85+
sprintf("\nLog files in %s cleared.\n", clean_path(WRITEPATH . 'logs')),
86+
preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer()),
87+
);
8588
}
8689

87-
public function testClearLogsAbortsClearWithoutForce(): void
90+
public function testClearLogsCancelsWithoutForce(): void
8891
{
8992
$this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log");
9093

@@ -96,16 +99,19 @@ public function testClearLogsAbortsClearWithoutForce(): void
9699

97100
$this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log");
98101
$this->assertSame(
99-
<<<'EOT'
100-
Are you sure you want to delete the logs? [n, y]: n
101-
Deleting logs aborted.
102-
103-
EOT,
102+
sprintf(
103+
<<<'EOT'
104+
Delete all log files in %s? [n, y]: n
105+
Log deletion cancelled.
106+
107+
EOT,
108+
clean_path(WRITEPATH . 'logs'),
109+
),
104110
preg_replace('/\e\[[^m]+m/', '', $io->getOutput()),
105111
);
106112
}
107113

108-
public function testClearLogsAbortsClearWithoutForceWithDefaultAnswer(): void
114+
public function testClearLogsCancelsWithoutForceWithDefaultAnswer(): void
109115
{
110116
$this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log");
111117

@@ -119,17 +125,20 @@ public function testClearLogsAbortsClearWithoutForceWithDefaultAnswer(): void
119125

120126
$this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log");
121127
$this->assertSame(
122-
<<<EOT
123-
Are you sure you want to delete the logs? [n, y]:{$space}
124-
Deleting logs aborted.
125-
126-
EOT,
128+
sprintf(
129+
<<<EOT
130+
Delete all log files in %s? [n, y]:{$space}
131+
Log deletion cancelled.
132+
133+
EOT,
134+
clean_path(WRITEPATH . 'logs'),
135+
),
127136
preg_replace('/\e\[[^m]+m/', '', $io->getOutput()),
128137
);
129138
}
130139

131-
#[DataProvider('provideClearLogsAbortsNonInteractivelyAndHintsAboutForceFlag')]
132-
public function testClearLogsAbortsNonInteractivelyAndHintsAboutForceFlag(string $flag): void
140+
#[DataProvider('provideClearLogsAbortsNonInteractively')]
141+
public function testClearLogsAbortsNonInteractively(string $flag): void
133142
{
134143
$this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log");
135144

@@ -139,8 +148,7 @@ public function testClearLogsAbortsNonInteractivelyAndHintsAboutForceFlag(string
139148
$this->assertSame(
140149
<<<'EOT'
141150
142-
Deleting logs aborted.
143-
If you want, use the "--force" option to force delete all log files.
151+
Log deletion aborted: pass --force to delete log files in non-interactive mode.
144152

145153
EOT,
146154
preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer()),
@@ -150,7 +158,7 @@ public function testClearLogsAbortsNonInteractivelyAndHintsAboutForceFlag(string
150158
/**
151159
* @return iterable<string, array{string}>
152160
*/
153-
public static function provideClearLogsAbortsNonInteractivelyAndHintsAboutForceFlag(): iterable
161+
public static function provideClearLogsAbortsNonInteractively(): iterable
154162
{
155163
yield 'long form' => ['--no-interaction'];
156164

@@ -169,11 +177,14 @@ public function testClearLogsWithoutForceButWithConfirmation(): void
169177

170178
$this->assertFileDoesNotExist(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log");
171179
$this->assertSame(
172-
<<<'EOT'
173-
Are you sure you want to delete the logs? [n, y]: y
174-
Logs cleared.
175-
176-
EOT,
180+
sprintf(
181+
<<<'EOT'
182+
Delete all log files in %1$s? [n, y]: y
183+
Log files in %1$s cleared.
184+
185+
EOT,
186+
clean_path(WRITEPATH . 'logs'),
187+
),
177188
preg_replace('/\e\[[^m]+m/', '', $io->getOutput()),
178189
);
179190
}
@@ -194,7 +205,7 @@ public function testClearLogsFailsOnChmodFailure(): void
194205

195206
$this->assertFileExists($path);
196207
$this->assertSame(
197-
"\nError in deleting the logs files.\n",
208+
sprintf("\nFailed to delete log files in %s.\n", clean_path(WRITEPATH . 'logs')),
198209
preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer()),
199210
);
200211
}

‎user_guide_src/source/changelogs/v4.8.0.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ Behavior Changes
3030
- **Commands:** The ``filter:check`` command now requires the HTTP method argument to be uppercase (e.g., ``spark filter:check GET /`` instead of ``spark filter:check get /``).
3131
- **Commands:** Several built-in commands have been migrated from ``BaseCommand`` to the modern ``AbstractCommand`` style. Applications that extend a built-in command to override
3232
behaviour may need to re-implement against the modern API (``configure()`` + ``execute()`` and the ``#[Command]`` attribute) once the class it extends is migrated, or, preferably, compose instead of extending. Invocations on the command line are unaffected.
33+
- **Commands:** The ``logs:clear`` command now returns ``EXIT_SUCCESS`` (previously ``EXIT_ERROR``) when the user declines the interactive confirmation prompt, since user-initiated cancellation is not a failure. Output messages have also been reworded to distinguish cancellation (interactive ``n``) from abort (non-interactive without ``--force``), and the resolved log directory path is now included in the prompt, success, and failure messages.
3334
- **Database:** The Postgre driver's ``$db->error()['code']`` previously always returned ``''``. It now returns the 5-character SQLSTATE string for query and transaction failures (e.g., ``'42P01'``), or ``'08006'`` for connection-level failures. Code that relied on ``$db->error()['code'] === ''`` will need updating.
3435
- **Filters:** HTTP method matching for method-based filters is now case-sensitive. The keys in ``Config\Filters::$methods`` must exactly match the request method
3536
(e.g., ``GET``, ``POST``). Lowercase method names (e.g., ``post``) will no longer match.

0 commit comments

Comments
 (0)