Skip to content
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

Hide error output when printing has been disabled #921

Closed
wants to merge 1 commit into from

Conversation

ipeevski
Copy link
Contributor

@ipeevski ipeevski commented Jan 9, 2020

Overview

This pull request:

  • Fixes a bug
  • Adds a feature
  • Breaks backwards compatibility
  • Has tests that cover changes
  • Adds or fixes documentation

Summary

Hide error output when printing has been disabled

Description

Nothing should be printed if printing is disabled.
Before, successful messages were being hidden, but error exit codes were being printed out, even if $task->silent(true);

Nothing should be printed if printing is disabled.
Before, successful messages were being hidden, but error exit codes were being printed out, even if `$task->silent(true);`
@ipeevski
Copy link
Contributor Author

ipeevski commented Jan 9, 2020

It doesn't look like the CI failure is related to the change?!

@greg-1-anderson
Copy link
Member

AppVeyor has started the annoying practice of providing only ONE version of PHP at a time. They recently changed their one available version from PHP 7.3 to PHP 7.4, which is what broke the appveryor tests here.

@greg-1-anderson
Copy link
Member

@DavertMik Does this break any of your use-cases? Do you accept this is a bugfix, and not a behavior change?

@ipeevski
Copy link
Contributor Author

ipeevski commented Jan 9, 2020

I don't claim to know all the use cases of course ... but to clarify, if I want to hide all the Robo generated output (and only have my own via ->say() or old fashioned echo), there is no way to do that. I can hide output if the command was successful, but errors still show up.
If I pass in -q to the command line, it has a different side effect that ->say() calls disappear too (although echo ones still work).

What I would expect:

  • call to ->silent() stops all internal Robo generated output. Including when command is run, what was it's return code, etc. It should keep visible explicit calls to ->say() etc (which it does already).
  • -q I would expect does the same as the described behaviour above, but don't really mind if it keeps its current behaviour, as long as the issue in the suggested pull request is resolved.

Let me know if I can give further clarifications. My explanation is perhaps a bit convoluted.

@greg-1-anderson
Copy link
Member

@ipeevski: I understand your use-case. I think this is a good idea for the next major release of Robo. If we want to put it in the current Robo release, we have to make sure that it does not break anyone's existing use cases. Technically this is a breaking change, unless this is a bug, as reported.

I just want the primary maintainer to weigh in and agree this is a bug.

@ipeevski
Copy link
Contributor Author

All good. Let's see what he says.

@greg-1-anderson greg-1-anderson deleted the branch consolidation:master February 21, 2021 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants