Skip to content

Conversation

@devoncarew
Copy link
Contributor

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
Copy link
Contributor Author

devoncarew commented Feb 12, 2018

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

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)");
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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:');
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

extraneous comma

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@Hixie
Copy link
Contributor

Hixie commented Feb 13, 2018

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.

@devoncarew
Copy link
Contributor Author

devoncarew commented Feb 13, 2018

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
Copy link
Contributor

zoechi commented Feb 14, 2018

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
Copy link
Contributor

Hixie commented Feb 15, 2018

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

@Hixie
Copy link
Contributor

Hixie commented Feb 15, 2018

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

@Hixie
Copy link
Contributor

Hixie commented Feb 15, 2018

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
Copy link
Contributor

Hixie commented Feb 15, 2018

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
Copy link
Member

xster commented Feb 15, 2018

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

@Hixie
Copy link
Contributor

Hixie commented Feb 15, 2018

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

@xster
Copy link
Member

xster commented Feb 15, 2018

I think it detaches without killing the client process

@devoncarew
Copy link
Contributor Author

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.

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

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

why do we not need this check?

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)");
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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

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'.");
Copy link
Contributor

Choose a reason for hiding this comment

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

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

@Hixie
Copy link
Contributor

Hixie commented Mar 9, 2018

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
Copy link
Contributor Author

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.

@devoncarew devoncarew closed this Mar 20, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants