-
Notifications
You must be signed in to change notification settings - Fork 28.1k
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
[et] Allow disabling the "cute" output mode, so folks can grep
the logs
#147903
Comments
Oh man, well tools like I think making |
Ah, I just filed #147936 which is a duplicate. We need to check that stdout is a terminal type and disable the pretty printing if its not. It is also ok to allow manually disabling pretty printing to terminal types too. |
As an aside, we need to go a bit out our way to fool Xcode since it pretends to be a terminal but can't handle being a terminal quite right. GN does this detection. |
https://api.dart.dev/stable/3.3.4/dart-io/Stdout/hasTerminal.html may be what we need (and the one for stderr). |
… provided. (#52619) Closes flutter/flutter#147894. While doing this PR I realized we were basically passing `(bool verbose, Envrionment)` as a tuple around, so I just moved the concept _into_ `Environment` directly, and made the necessary code changes across the tool and tests. To clarify, this does _not_ mimic the output of `ninja --verbose` _today_, because we also don't stream the output directly, and instead do terminal magic. Combined with a hypothetical fix for flutter/flutter#147903, this would work exactly the same as before. /cc @loic-sharma @johnmccutchan
Update: We do some checks for |
When I do: flutter % et build -v | grep supportsAnsiEscapes
supportsAnsiEscapes: false | hasTerminal: false ... it appears detection does work, so I'm assuming we're not guarding something we should be. |
The fix is as simple as: diff --git a/tools/engine_tool/lib/src/logger.dart b/tools/engine_tool/lib/src/logger.dart
index 28bd847885..8b4565d94b 100644
--- a/tools/engine_tool/lib/src/logger.dart
+++ b/tools/engine_tool/lib/src/logger.dart
@@ -169,6 +169,8 @@ class Logger {
/// and emits a carriage return.
void clearLine() {
if (!io.stdout.hasTerminal || _test) {
+ // Just emit a newline in tests or when not connected to a terminal.
+ _ioSinkWrite(io.stdout, '\n');
return;
} However, it is next to impossible to test because the Logger does a bunch of things with global state. I'm going to provide a bit more TLC to |
… updates. (#52681) For illustrative purposes: ```sh $ et build | grep '.*' ``` ... should still get line-per-line status updates, but it does not without this patch. It's hard to write tests because of global state, so I've declined to do so at the moment. Closes flutter/flutter#147903.
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
Forked from #145711 (comment).
It should be reasonable to do something like:
We could make this yet-another flag, i.e.
--no-animations
, but I think verbose implying it is also fine?Wdut?
/cc @gaaclarke
The text was updated successfully, but these errors were encountered: