Skip to content

[flutter_tools] inject output preferences at the top level - #58875

Merged
jonahwilliams merged 5 commits into
flutter:masterfrom
jonahwilliams:lift_output_preferences
Jun 10, 2020
Merged

jonahwilliams merged 5 commits into
flutter:masterfrom
jonahwilliams:lift_output_preferences

Conversation

@jonahwilliams

@jonahwilliams jonahwilliams commented Jun 6, 2020 •

Copy link
Copy Markdown
Contributor

Description

The tool was setting the output preferences in a sub-context. Originally these were not injected before the arg parsers were created, though that was fixed by the lazy command creation. Once local engine is removed, the inner flutter_command Zone can be removed.

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 6, 2020
@jonahwilliams

Copy link
Copy Markdown
Contributor Author

This will also work towards unblocking testWithoutContext when using TestFlutterCommandRunner

expect(preferences.showColor, true);
expect(preferences.wrapText, false);
// this calls context
// expect(preferences.wrapColumn, 80);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this supposed to be commented out?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

}
}

// If we're not writing to a terminal with a defined width, then don't wrap

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the logic changed and this comment is no longer accurate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, and added test case that we default to 100 if terminalColumns is null

@christopherfujino christopherfujino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jonahwilliams
jonahwilliams merged commit dca6320 into flutter:master Jun 10, 2020
@jonahwilliams
jonahwilliams deleted the lift_output_preferences branch June 10, 2020 19:13
jmagman added a commit that referenced this pull request Jun 10, 2020
jmagman added a commit that referenced this pull request Jun 10, 2020
@jonahwilliams
jonahwilliams restored the lift_output_preferences branch June 11, 2020 16:20
zljj0818 pushed a commit to zljj0818/flutter that referenced this pull request Jun 22, 2020
…8875)

The tool was setting the output preferences in a sub-context. Originally these were not injected before the arg parsers were created, though that was fixed by the lazy command creation. Once local engine is removed, the inner flutter_command Zone can be removed.
zljj0818 pushed a commit to zljj0818/flutter that referenced this pull request Jun 22, 2020
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
…8875)

The tool was setting the output preferences in a sub-context. Originally these were not injected before the arg parsers were created, though that was fixed by the lazy command creation. Once local engine is removed, the inner flutter_command Zone can be removed.
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants