refactor the flutter run detailed help message by devoncarew · Pull Request #14651 · flutter/flutter

devoncarew · 2018-02-12T21:21:39Z

The 'h' help message from flutter run has gotten pretty long. I'm looking to add one more item to it, so want to take the opportunity to re-structure the output a bit before hand.

current short-form (unchanged):

🔥  To hot reload your app on the fly, press "r". To restart the app entirely, press "R".
An Observatory debugger and profiler on iPhone 6 is available at: http://127.0.0.1:8100/
For a more detailed help message, press "h". To quit, press "q".

previous long-form help:

🔥  To hot reload your app on the fly, press "r". To restart the app entirely, press "R".
An Observatory debugger and profiler on iPhone 6 is available at: http://127.0.0.1:8100/
You can dump the widget hierarchy of the app (debugDumpApp) by pressing "w".
To dump the rendering tree of the app (debugDumpRenderTree), press "t".
For layers (debugDumpLayerTree), use "L"; for accessibility (debugDumpSemantics), use "S" (for geometric order) or "U" (for inverse hit test order).
To toggle the widget inspector (WidgetsApp.showWidgetInspectorOverride), press "i".
To toggle the display of construction lines (debugPaintSizeEnabled), press "p".
To simulate different operating systems, (defaultTargetPlatform), press "o".
To display the performance overlay (WidgetsApp.showPerformanceOverlay), press "P".
To save a screenshot to flutter.png, press "s".
To repeat this help message, press "h". To quit, press "q".

updated long-form help:

Reloading:
🔥  To hot reload your app on the fly, press "r"; to restart the app entirely, press "R".

On device UI:
- 'i': toggle the widget inspector (showWidgetInspector)
- 'P': display the performance overlay (showPerformanceOverlay)

Widgets, layers, render and semantic trees:
- 'w': dump the widget hierarchy of the app (debugDumpApp)
- 't': dump the rendering tree of the app (debugDumpRenderTree)
- 'L': dump the layer tree (debugDumpLayerTree)
- 'S': dump accessibility in geometric order; 'U' for inverse hit test order (debugDumpSemantics)

Flutter toggles and other:
- 'p': toggle the display of construction lines (debugPaintSizeEnabled)
- 'o': simulate different operating systems, (defaultTargetPlatform)
- 's': take a screenshot (saved to flutter_XX.png)

To repeat this help message, press "h"; to quit, press "q".

devoncarew · 2018-02-12T21:22:41Z

Looking over that help text, I may update the above to remove the leading '- ' (and just use spaces to indent).

Hixie · 2018-02-13T01:10:03Z

packages/flutter_tools/lib/src/resident_runner.dart

+    printStatus("- 't': dump the rendering tree of the app (debugDumpRenderTree)");
+    if (isRunningDebug)
+      printStatus("- 'L': dump the layer tree (debugDumpLayerTree)");
+    printStatus("- 'S': dump accessibility in geometric order; 'U' for inverse hit test order (debugDumpSemantics)");


I'd probably put these on two lines at this point. That would let you be less terse.

Hixie · 2018-02-13T01:10:44Z

packages/flutter_tools/lib/src/resident_runner.dart

+      printStatus("- 'L': dump the layer tree (debugDumpLayerTree)");
+    printStatus("- 'S': dump accessibility in geometric order; 'U' for inverse hit test order (debugDumpSemantics)");
+    printStatus('');
+    printStatus('Flutter toggles and other:');


that title is going to look ugly in release builds, where there's only one command here. :-)

Hixie · 2018-02-13T01:10:54Z

packages/flutter_tools/lib/src/resident_runner.dart

+    if (isRunningDebug)
+      printStatus("- 'p': toggle the display of construction lines (debugPaintSizeEnabled)");
+    if (isRunningDebug)
+      printStatus("- 'o': simulate different operating systems, (defaultTargetPlatform)");


extraneous comma

Hixie · 2018-02-13T01:11:49Z

Can you add tests that verify the exact output for release, profile, and debug? Or at least dump the output here so we can look at it.

…e, and --release

devoncarew · 2018-02-13T22:15:09Z

Updated based on feedback, and reviewing the --debug, --profile, and --release help text:

--debug:

Reloading:
🔥  To hot reload your app on the fly, press 'r'; to restart the app entirely, press 'R'.

On device UI:
- 'i': toggle the widget inspector (showWidgetInspector)
- 'P': display the performance overlay (showPerformanceOverlay)

Widgets, layers, render and semantic trees:
- 'w': dump the widget hierarchy of the app (debugDumpApp)
- 't': dump the rendering tree of the app (debugDumpRenderTree)
- 'L': dump the layer tree (debugDumpLayerTree)
- 'S': dump accessibility in geometric order (debugDumpSemantics)
- 'U': dump accessibility in inverse hit test order

Flutter framework toggles:
- 'p': toggle the display of construction lines (debugPaintSizeEnabled)
- 'o': simulate different operating systems (defaultTargetPlatform)
- 's': take a screenshot (saved to flutter_XX.png)

To repeat this help message, press 'h'; to quit, press 'q'.

--profile:

On device UI:
- 'P': display the performance overlay (showPerformanceOverlay)

Widgets, layers, render and semantic trees:
- 'w': dump the widget hierarchy of the app (debugDumpApp)
- 't': dump the rendering tree of the app (debugDumpRenderTree)
- 'S': dump accessibility in geometric order (debugDumpSemantics)
- 'U': dump accessibility in inverse hit test order

Flutter framework toggles:
- 's': take a screenshot (saved to flutter_XX.png)

To repeat this help message, press 'h'; to quit, press 'q'.

--release:

To repeat this help message, press 'h'; to quit, press 'q'.

zoechi · 2018-02-14T10:11:18Z

What about

Reloading:
🔥 'r': hot reload your app on the fly.
🔥 'R': restart the app entirely.

instead of

Reloading:
🔥  To hot reload your app on the fly, press 'r'; to restart the app entirely, press 'R'.

?

Hixie · 2018-02-15T00:27:14Z

"On device UI" feels weird, maybe "Inspectors"?

Hixie · 2018-02-15T00:28:02Z

Rather than "dump accessibility" maybe "dump accessibility tree" for consistency with the others.

Hixie · 2018-02-15T00:29:11Z

If the only thing we show in release builds is the line saying how to get the help, then maybe replace that with just "Development tools are not built into release builds, so no commands are available. For hot reload, inspectors, and other development tools, consider using a debug build."

Hixie · 2018-02-15T00:30:47Z

I'd probably merge "Flutter framework toggles" into the "On device UI" section.
Also, the screenshot thing is not a framework toggle, so it's weird (especially in --profile mode) that that is where that option is.

You know, in general maybe the section headers aren't worth it. Maybe omit them entirely?

xster · 2018-02-15T20:20:37Z

The 'd' command would be interesting to show too I think

Hixie · 2018-02-15T21:55:35Z

We should definitely show everything. What is 'd'? :-)

xster · 2018-02-15T21:56:38Z

I think it detaches without killing the client process

devoncarew · 2018-02-19T18:24:15Z

Updated!

On device UI: ==> Inspectors:
Flutter framework toggles: ==> Framework toggles and utilities: (to make it a better place to host the screenshot command)
dump accessibility ==> dump accessibility tree

And:

You know, in general maybe the section headers aren't worth it. Maybe omit them entirely?

I'd like to keep them - I think the help text here could use some structure, at least in --debug mode, given how many there are. The --debug mode has the most options, and is the mode users will be running in the most, so optimizing the UI for that mode makes sense.

There are other changes we could do here, but I'd like to same additional work for future PRs.

Hixie · 2018-03-09T22:28:44Z

packages/flutter_tools/lib/src/resident_runner.dart

+    printStatus('Framework toggles and utilities:');
+    if (isRunningDebug) {
+      printStatus(
+          "- 'p': toggle the display of construction lines (debugPaintSizeEnabled)");


nit: let's avoid wrapping here, to keep the code consistent (makes the reader less likely to wonder why some are different than others)

Hixie · 2018-03-09T22:29:15Z

packages/flutter_tools/lib/src/resident_runner.dart

+      printStatus(
+          "- 'o': simulate different operating systems (defaultTargetPlatform)");
    }
-    if (flutterDevices.any((FlutterDevice d) => d.device.supportsScreenshot))


why do we not need this check?

Hixie · 2018-03-09T22:30:16Z

packages/flutter_tools/lib/src/resident_runner.dart

+    printStatus("- 'w': dump the widget hierarchy of the app (debugDumpApp)");
+    printStatus("- 't': dump the rendering tree of the app (debugDumpRenderTree)");
+    if (isRunningDebug)
+      printStatus("- 'L': dump the layer tree (debugDumpLayerTree)");


as far as i can tell, this works fine in non-debug mode too

Hixie · 2018-03-09T22:32:23Z

packages/flutter_tools/lib/src/run_hot.dart

+    if (details)
+      printStatus('Reloading:');
+      printStatus(
+        "$fire  To hot reload your app on the fly, press 'r'; to restart the app entirely, press 'R'.",


we want this message even if details is false. We found in UX studies that it was critical.

Hixie · 2018-03-09T22:33:40Z

packages/flutter_tools/lib/src/run_hot.dart

    if (details) {
      printHelpDetails();
-      printStatus('To repeat this help message, press "h". To quit, press "q".');
+      printStatus("To repeat this help message, press 'h'; to quit, press 'q'.");


We should probably use the same format for 'h', 'q', and 'd' as we do for everything else, in details mode.

Hixie · 2018-03-09T22:36:20Z

Can you add some tests that check what the output is in each of these cases? Such tests will make it much easier for future reviewers to review changes to this code.

default text that is shown on startup for debug mode, profile mode, and release mode
help text that is shown when pressing 'h' in debug mode and profile mode
help shown in release mode when screenshots are supported and when they are not
help shown in debug mode with two devices
help shown in release mode with two devices

devoncarew · 2018-03-20T20:38:06Z

Can you add some tests that check what the output is in each of these cases? Such tests will make it much easier for future reviewers to review changes to this code.

Sounds good; I don't have the cycles to address right now. I'll close this PR in the interest of clearing up the review queue, and file an issues to track improvements to the help message.

refactor flutter run detailed help message

df588f3

googlebot added the cla: yes label Feb 12, 2018

Hixie reviewed Feb 13, 2018

View reviewed changes

devoncarew added 2 commits February 13, 2018 13:07

Merge branch 'master' into refactor_run_help

64b605e

additional changes from review and checking against --debug, --profil…

4b0f70a

…e, and --release

review comments

1c29d42

Hixie reviewed Mar 9, 2018

View reviewed changes

devoncarew closed this Mar 20, 2018

devoncarew mentioned this pull request Mar 20, 2018

improve the help message from flutter run #15762

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2021

Conversation

devoncarew commented Feb 12, 2018

Uh oh!

devoncarew commented Feb 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hixie commented Feb 13, 2018

Uh oh!

devoncarew commented Feb 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zoechi commented Feb 14, 2018

Uh oh!

Hixie commented Feb 15, 2018

Uh oh!

Hixie commented Feb 15, 2018

Uh oh!

Hixie commented Feb 15, 2018

Uh oh!

Hixie commented Feb 15, 2018

Uh oh!

xster commented Feb 15, 2018

Uh oh!

Hixie commented Feb 15, 2018

Uh oh!

xster commented Feb 15, 2018

Uh oh!

devoncarew commented Feb 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hixie commented Mar 9, 2018

Uh oh!

devoncarew commented Mar 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

devoncarew commented Feb 12, 2018 •

edited

Loading

devoncarew commented Feb 13, 2018 •

edited

Loading