Unverified Commit 4559ae1a authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Stop wrapping adb, gradle and ios logger output, and update terminal wrapping...

Stop wrapping adb, gradle and ios logger output, and update terminal wrapping column dynamically. (#23592)

Subcommand output (gradle, adb, etc) is no longer wrapped, and wrapping notices when the terminal column width changes dynamically now.

Fixes #23267.
Fixes #23266.
parent be7226f0
......@@ -440,7 +440,7 @@ class AndroidDevice extends Device {
final String result = (await runCheckedAsync(cmd)).stdout;
// This invocation returns 0 even when it fails.
if (result.contains('Error: ')) {
printError(result.trim());
printError(result.trim(), wrap: false);
return LaunchResult.failed();
}
......
......@@ -34,7 +34,9 @@ Future<void> buildApk({
final List<String> validationResult = androidSdk.validateSdkWellFormed();
if (validationResult.isNotEmpty) {
validationResult.forEach(printError);
for (String message in validationResult) {
printError(message, wrap: false);
}
throwToolExit('Try re-installing or updating your Android SDK.');
}
......
......@@ -40,6 +40,8 @@ abstract class Logger {
/// If [hangingIndent] is specified, then any wrapped lines will be indented
/// by this much more than the first line, if wrapping is enabled in
/// [outputPreferences].
/// If [wrap] is specified, then it overrides the
/// [outputPreferences.wrapText] setting.
void printError(
String message, {
StackTrace stackTrace,
......@@ -47,6 +49,7 @@ abstract class Logger {
TerminalColor color,
int indent,
int hangingIndent,
bool wrap,
});
/// Display normal output of the command. This should be used for things like
......@@ -68,6 +71,8 @@ abstract class Logger {
/// If [hangingIndent] is specified, then any wrapped lines will be indented
/// by this much more than the first line, if wrapping is enabled in
/// [outputPreferences].
/// If [wrap] is specified, then it overrides the
/// [outputPreferences.wrapText] setting.
void printStatus(
String message, {
bool emphasis,
......@@ -75,6 +80,7 @@ abstract class Logger {
bool newline,
int indent,
int hangingIndent,
bool wrap,
});
/// Use this for verbose tracing output. Users can turn this output on in order
......@@ -111,9 +117,10 @@ class StdoutLogger extends Logger {
TerminalColor color,
int indent,
int hangingIndent,
bool wrap,
}) {
message ??= '';
message = wrapText(message, indent: indent, hangingIndent: hangingIndent);
message = wrapText(message, indent: indent, hangingIndent: hangingIndent, shouldWrap: wrap);
_status?.cancel();
_status = null;
if (emphasis == true)
......@@ -133,9 +140,10 @@ class StdoutLogger extends Logger {
bool newline,
int indent,
int hangingIndent,
bool wrap,
}) {
message ??= '';
message = wrapText(message, indent: indent, hangingIndent: hangingIndent);
message = wrapText(message, indent: indent, hangingIndent: hangingIndent, shouldWrap: wrap);
_status?.cancel();
_status = null;
if (emphasis == true)
......@@ -232,9 +240,10 @@ class BufferLogger extends Logger {
TerminalColor color,
int indent,
int hangingIndent,
bool wrap,
}) {
_error.writeln(terminal.color(
wrapText(message, indent: indent, hangingIndent: hangingIndent),
wrapText(message, indent: indent, hangingIndent: hangingIndent, shouldWrap: wrap),
color ?? TerminalColor.red,
));
}
......@@ -247,11 +256,12 @@ class BufferLogger extends Logger {
bool newline,
int indent,
int hangingIndent,
bool wrap,
}) {
if (newline != false)
_status.writeln(wrapText(message, indent: indent, hangingIndent: hangingIndent));
_status.writeln(wrapText(message, indent: indent, hangingIndent: hangingIndent, shouldWrap: wrap));
else
_status.write(wrapText(message, indent: indent, hangingIndent: hangingIndent));
_status.write(wrapText(message, indent: indent, hangingIndent: hangingIndent, shouldWrap: wrap));
}
@override
......@@ -297,10 +307,11 @@ class VerboseLogger extends Logger {
TerminalColor color,
int indent,
int hangingIndent,
bool wrap,
}) {
_emit(
_LogType.error,
wrapText(message, indent: indent, hangingIndent: hangingIndent),
wrapText(message, indent: indent, hangingIndent: hangingIndent, shouldWrap: wrap),
stackTrace,
);
}
......@@ -313,8 +324,9 @@ class VerboseLogger extends Logger {
bool newline,
int indent,
int hangingIndent,
bool wrap,
}) {
_emit(_LogType.status, wrapText(message, indent: indent, hangingIndent: hangingIndent));
_emit(_LogType.status, wrapText(message, indent: indent, hangingIndent: hangingIndent, shouldWrap: wrap));
}
@override
......
......@@ -148,7 +148,7 @@ Future<int> runCommandAndStreamOutput(List<String> cmd, {
if (trace)
printTrace(message);
else
printStatus(message);
printStatus(message, wrap: false);
}
});
final StreamSubscription<String> stderrSubscription = process.stderr
......@@ -159,7 +159,7 @@ Future<int> runCommandAndStreamOutput(List<String> cmd, {
if (mapFunction != null)
line = mapFunction(line);
if (line != null)
printError('$prefix$line');
printError('$prefix$line', wrap: false);
});
// Wait for stdout to be fully processed
......
......@@ -45,7 +45,7 @@ class OutputPreferences {
int wrapColumn,
bool showColor,
}) : wrapText = wrapText ?? io.stdio?.hasTerminal ?? const io.Stdio().hasTerminal,
wrapColumn = wrapColumn ?? io.stdio?.terminalColumns ?? const io.Stdio().terminalColumns ?? kDefaultTerminalColumns,
_overrideWrapColumn = wrapColumn,
showColor = showColor ?? platform.stdoutSupportsAnsi ?? false;
/// If [wrapText] is true, then any text sent to the context's [Logger]
......@@ -64,8 +64,12 @@ class OutputPreferences {
/// To find out if we're writing to a terminal, it tries the context's stdio,
/// and if that's not set, it tries creating a new [io.Stdio] and asks it, if
/// that doesn't have an idea of the terminal width, then we just use a
/// default of 100. It will be ignored if wrapText is false.
final int wrapColumn;
/// default of 100. It will be ignored if [wrapText] is false.
final int _overrideWrapColumn;
int get wrapColumn {
return _overrideWrapColumn ?? io.stdio?.terminalColumns
?? const io.Stdio().terminalColumns ?? kDefaultTerminalColumns;
}
/// Whether or not to output ANSI color codes when writing to the output
/// terminal. Defaults to whatever [platform.stdoutSupportsAnsi] says if
......
......@@ -313,7 +313,8 @@ const int kMinColumnWidth = 10;
/// Wraps a block of text into lines no longer than [columnWidth].
///
/// Tries to split at whitespace, but if that's not good enough to keep it
/// under the limit, then it splits in the middle of a word.
/// under the limit, then it splits in the middle of a word. If [columnWidth] is
/// smaller than 10 columns, will wrap at 10 columns.
///
/// Preserves indentation (leading whitespace) for each line (delimited by '\n')
/// in the input, and will indent wrapped lines that same amount, adding
......@@ -339,11 +340,12 @@ const int kMinColumnWidth = 10;
/// [outputPreferences.wrapColumn], which is set with the --wrap-column option.
///
/// If [outputPreferences.wrapText] is false, then the text will be returned
/// unchanged.
/// unchanged. If [shouldWrap] is specified, then it overrides the
/// [outputPreferences.wrapText] setting.
///
/// The [indent] and [hangingIndent] must be smaller than [columnWidth] when
/// added together.
String wrapText(String text, {int columnWidth, int hangingIndent, int indent}) {
String wrapText(String text, {int columnWidth, int hangingIndent, int indent, bool shouldWrap}) {
if (text == null || text.isEmpty) {
return '';
}
......@@ -366,6 +368,7 @@ String wrapText(String text, {int columnWidth, int hangingIndent, int indent}) {
final List<String> firstLineWrap = _wrapTextAsLines(
trimmedText,
columnWidth: columnWidth - leadingWhitespace.length,
shouldWrap: shouldWrap,
);
notIndented = <String>[firstLineWrap.removeAt(0)];
trimmedText = trimmedText.substring(notIndented[0].length).trimLeft();
......@@ -373,12 +376,14 @@ String wrapText(String text, {int columnWidth, int hangingIndent, int indent}) {
notIndented.addAll(_wrapTextAsLines(
trimmedText,
columnWidth: columnWidth - leadingWhitespace.length - hangingIndent,
shouldWrap: shouldWrap,
));
}
} else {
notIndented = _wrapTextAsLines(
trimmedText,
columnWidth: columnWidth - leadingWhitespace.length,
shouldWrap: shouldWrap,
);
}
String hangingIndentString;
......@@ -426,14 +431,16 @@ class _AnsiRun {
/// default will be [outputPreferences.wrapColumn].
///
/// If [outputPreferences.wrapText] is false, then the text will be returned
/// simply split at the newlines, but not wrapped.
List<String> _wrapTextAsLines(String text, {int start = 0, int columnWidth}) {
/// simply split at the newlines, but not wrapped. If [shouldWrap] is specified,
/// then it overrides the [outputPreferences.wrapText] setting.
List<String> _wrapTextAsLines(String text, {int start = 0, int columnWidth, bool shouldWrap}) {
if (text == null || text.isEmpty) {
return <String>[''];
}
assert(columnWidth != null);
assert(columnWidth >= 0);
assert(start >= 0);
shouldWrap ??= outputPreferences.wrapText;
/// Returns true if the code unit at [index] in [text] is a whitespace
/// character.
......@@ -495,7 +502,7 @@ List<String> _wrapTextAsLines(String text, {int start = 0, int columnWidth}) {
for (String line in text.split('\n')) {
// If the line is short enough, even with ANSI codes, then we can just add
// add it and move on.
if (line.length <= effectiveLength || !outputPreferences.wrapText) {
if (line.length <= effectiveLength || !shouldWrap) {
result.add(line);
continue;
}
......
......@@ -758,6 +758,7 @@ class NotifyingLogger extends Logger {
TerminalColor color,
int indent,
int hangingIndent,
bool wrap,
}) {
_messageController.add(LogMessage('error', message, stackTrace));
}
......@@ -770,6 +771,7 @@ class NotifyingLogger extends Logger {
bool newline = true,
int indent,
int hangingIndent,
bool wrap,
}) {
_messageController.add(LogMessage('status', message));
}
......@@ -892,6 +894,7 @@ class _AppRunLogger extends Logger {
TerminalColor color,
int indent,
int hangingIndent,
bool wrap,
}) {
if (parent != null) {
parent.printError(
......@@ -900,6 +903,7 @@ class _AppRunLogger extends Logger {
emphasis: emphasis,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
);
} else {
if (stackTrace != null) {
......@@ -925,6 +929,7 @@ class _AppRunLogger extends Logger {
bool newline = true,
int indent,
int hangingIndent,
bool wrap,
}) {
if (parent != null) {
parent.printStatus(
......@@ -934,6 +939,7 @@ class _AppRunLogger extends Logger {
newline: newline,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
);
} else {
_sendLogEvent(<String, dynamic>{'log': message});
......
......@@ -540,7 +540,7 @@ class FuchsiaDeviceCommandRunner {
printTrace(args.join(' '));
final ProcessResult result = await processManager.run(args);
if (result.exitCode != 0) {
printStatus('Command failed: $command\nstdout: ${result.stdout}\nstderr: ${result.stderr}');
printStatus('Command failed: $command\nstdout: ${result.stdout}\nstderr: ${result.stderr}', wrap: false);
return null;
}
printTrace(result.stdout);
......
......@@ -51,7 +51,7 @@ class LogsCommand extends FlutterCommand {
// Start reading.
final StreamSubscription<String> subscription = logReader.logLines.listen(
printStatus,
(String message) => printStatus(message, wrap: false),
onDone: () {
exitCompleter.complete(0);
},
......
......@@ -342,7 +342,7 @@ class UpdatePackagesCommand extends FlutterCommand {
if (path != null)
buf.write(' <- ');
}
printStatus(buf.toString());
printStatus(buf.toString(), wrap: false);
}
if (paths.isEmpty) {
......
......@@ -27,6 +27,7 @@ void printError(
TerminalColor color,
int indent,
int hangingIndent,
bool wrap,
}) {
logger.printError(
message,
......@@ -35,6 +36,7 @@ void printError(
color: color,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
);
}
......@@ -54,6 +56,7 @@ void printStatus(
TerminalColor color,
int indent,
int hangingIndent,
bool wrap,
}) {
logger.printStatus(
message,
......@@ -62,6 +65,7 @@ void printStatus(
newline: newline ?? true,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
);
}
......
......@@ -240,7 +240,7 @@ class FlutterDevice {
return;
_loggingSubscription = device.getLogReader(app: package).logLines.listen((String line) {
if (!line.contains('Observatory listening on http'))
printStatus(line);
printStatus(line, wrap: false);
});
}
......
......@@ -253,9 +253,11 @@ class FlutterCommandRunner extends CommandRunner<void> {
contextOverrides[Logger] = VerboseLogger(logger);
}
final int terminalColumns = io.stdio.terminalColumns;
int wrapColumn = terminalColumns ?? kDefaultTerminalColumns;
if (topLevelResults['wrap-column'] != null) {
// Don't set wrapColumns unless the user said to: if it's set, then all
// wrapping will occur at this width explicitly, and won't adapt if the
// terminal size changes during a run.
int wrapColumn;
if (topLevelResults.wasParsed('wrap-column')) {
try {
wrapColumn = int.parse(topLevelResults['wrap-column']);
if (wrapColumn < 0) {
......@@ -272,7 +274,7 @@ class FlutterCommandRunner extends CommandRunner<void> {
// anything, unless the user explicitly said to.
final bool useWrapping = topLevelResults.wasParsed('wrap')
? topLevelResults['wrap']
: terminalColumns == null ? false : topLevelResults['wrap'];
: io.stdio.terminalColumns == null ? false : topLevelResults['wrap'];
contextOverrides[OutputPreferences] = OutputPreferences(
wrapText: useWrapping,
showColor: topLevelResults['color'],
......
......@@ -3,19 +3,22 @@
// found in the LICENSE file.
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/process.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:mockito/mockito.dart';
import 'package:process/process.dart';
import '../src/common.dart';
import '../src/context.dart';
import '../src/mocks.dart' show MockProcess, MockProcessManager;
void main() {
group('process exceptions', () {
ProcessManager mockProcessManager;
setUp(() {
mockProcessManager = MockProcessManager();
mockProcessManager = PlainMockProcessManager();
});
testUsingContext('runCheckedAsync exceptions should be ProcessException objects', () async {
......@@ -56,6 +59,35 @@ void main() {
expect(cleanup, 4);
});
});
group('output formatting', () {
MockProcessManager mockProcessManager;
BufferLogger mockLogger;
setUp(() {
mockProcessManager = MockProcessManager();
mockLogger = BufferLogger();
});
MockProcess Function(List<String>) processMetaFactory(List<String> stdout, {List<String> stderr = const <String>[]}) {
final Stream<List<int>> stdoutStream =
Stream<List<int>>.fromIterable(stdout.map<List<int>>((String s) => s.codeUnits));
final Stream<List<int>> stderrStream =
Stream<List<int>>.fromIterable(stderr.map<List<int>>((String s) => s.codeUnits));
return (List<String> command) => MockProcess(stdout: stdoutStream, stderr: stderrStream);
}
testUsingContext('Command output is not wrapped.', () async {
final List<String> testString = <String>['0123456789' * 10];
mockProcessManager.processFactory = processMetaFactory(testString, stderr: testString);
await runCommandAndStreamOutput(<String>['command']);
expect(mockLogger.statusText, equals('${testString[0]}\n'));
expect(mockLogger.errorText, equals('${testString[0]}\n'));
}, overrides: <Type, Generator>{
Logger: () => mockLogger,
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 40),
});
});
}
class MockProcessManager extends Mock implements ProcessManager {}
class PlainMockProcessManager extends Mock implements ProcessManager {}
\ No newline at end of file
......@@ -4,6 +4,7 @@
import 'dart:async';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/utils.dart';
import 'package:flutter_tools/src/base/version.dart';
import 'package:flutter_tools/src/base/terminal.dart';
......@@ -165,7 +166,8 @@ baz=qux
await Future<void>.delayed(kShortDelay);
}, kShortDelay);
final Duration duration = await completer.future;
expect(duration, greaterThanOrEqualTo(Duration(milliseconds: kShortDelay.inMilliseconds * 2)));
expect(
duration, greaterThanOrEqualTo(Duration(milliseconds: kShortDelay.inMilliseconds * 2)));
});
});
......@@ -200,14 +202,16 @@ baz=qux
const String _shortLine = 'Short line.';
const String _indentedLongLine = ' This is an indented long line that needs to be '
'wrapped and indentation preserved.';
final FakeStdio fakeStdio = FakeStdio();
void testWrap(String description, Function body) {
testUsingContext(description, body, overrides: <Type, Generator> {
void testWrap(String description, Function body) {
testUsingContext(description, body, overrides: <Type, Generator>{
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: _lineLength),
});
}
void testNoWrap(String description, Function body) {
testUsingContext(description, body, overrides: <Type, Generator> {
void testNoWrap(String description, Function body) {
testUsingContext(description, body, overrides: <Type, Generator>{
OutputPreferences: () => OutputPreferences(wrapText: false),
});
}
......@@ -215,6 +219,14 @@ baz=qux
test('does not wrap by default in tests', () {
expect(wrapText(_longLine), equals(_longLine));
});
testNoWrap('can override wrap preference if preference is off', () {
expect(wrapText(_longLine, columnWidth: _lineLength, shouldWrap: true), equals('''
This is a long line that needs to be
wrapped.'''));
});
testWrap('can override wrap preference if preference is on', () {
expect(wrapText(_longLine, shouldWrap: false), equals(_longLine));
});
testNoWrap('does not wrap at all if not told to wrap', () {
expect(wrapText(_longLine), equals(_longLine));
});
......@@ -226,6 +238,20 @@ baz=qux
This is a long line that needs to be
wrapped.'''));
});
testUsingContext('able to handle dynamically changing terminal column size', () {
fakeStdio.currentColumnSize = 20;
expect(wrapText(_longLine), equals('''
This is a long line
that needs to be
wrapped.'''));
fakeStdio.currentColumnSize = _lineLength;
expect(wrapText(_longLine), equals('''
This is a long line that needs to be
wrapped.'''));
}, overrides: <Type, Generator>{
OutputPreferences: () => OutputPreferences(wrapText: true),
Stdio: () => fakeStdio,
});
testWrap('wrap long lines with no whitespace', () {
expect(wrapText('0123456789' * 5, columnWidth: _lineLength), equals('''
0123456789012345678901234567890123456789
......@@ -335,3 +361,12 @@ needs to be wrapped.
});
});
}
class FakeStdio extends Stdio {
FakeStdio();
int currentColumnSize = 20;
@override
int get terminalColumns => currentColumnSize;
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment