-
-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add --output-file option to directly write formatted result output to… #629
Conversation
Hi thanks for the addition. Yes a test would be nice. Some implementations still lack coverage. And yes, I think you would need to use vfs to just create the files in memory. |
I added mikey179/vfsstream as a dev-dependency and wrote 2 tests for output-file and output-file + output-format. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. You can change the error message if you want. But not totally necessary 🙂
Thanks for the addition.
@@ -147,6 +162,24 @@ protected function execute(InputInterface $input, OutputInterface $output): int | |||
return $ignoreExitCode ? 0 : 1; | |||
} | |||
|
|||
if ($outputFile) { | |||
if (!is_writable(dirname($outputFile))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are checking if the folder is writeable. But the error message tells that the file is not writeable. Maybe we can clarify the error message a little?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Also added a test for this.
You also have conflicts now. Probably due to dependabot updates. |
Pushed a rebase. |
Thanks. Now tests are broken 😉 |
@icanhazstring rebased and fixed tests. Please try again! |
@icanhazstring hopefully last time - retry please ;-) I got a hardcoded path in the expected output, fixed it. |
Thanks. Merged 🙂 |
I tried to add this via Output classes, but this was a pain, since all the OutputFormatters rely on Symfony Console OutputInterface. Now I simply replace the $io SymfonyStyle with a new SymfonyStyle with BufferedOutput, which also removes all ansi codes on the fly.
A test can be added if needed, but I guess I would need to add something like vfsStream to avoid creating files while testing.
Didn't find tests for e.g. --output-format as well, so I don't know if this is necessary?
Comment would be appreciated.
All Submissions:
New Feature Submissions:
Changes to Core Features: