Skip to content
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

make the usage text more visible #3693

Merged
merged 2 commits into from May 3, 2016

Conversation

devoncarew
Copy link
Member

Use whitespace and indent to make the first-run usage text more visible:

[~/projects/flutter/flutter] flutter
Flutter - https://flutter.io - version 6072552868/master

  The Flutter tool anonymously reports feature usage statistics and basic crash reports to Google in
  order to help Google contribute improvements to Flutter over time. Use "flutter config" to control
  this behavior. See Google's privacy policy: https://www.google.com/intl/en/policies/privacy/

Manage your Flutter app development.

Usage: flutter <command> [arguments]

Global options:
-h, --help            Print this usage information.
-v, --verbose         Noisy logging, including all shell commands executed.
...

I experimented with changing where it displayed but had some issues with long and short running commands; happy to iterate on the look.

@Hixie

@Hixie
Copy link
Contributor

Hixie commented May 3, 2016

Some ideas to make it even more obvious:

╔═══════╗ 
║       ║
╚═══════╝ 

\e[0;1;41;37m - white on red text
\e[0;1;31m ... \e[0m bright red text, could be used for inside the box or outside it

Dunno, just some ideas. LGTM either way. If you do go the ANSI escape route, make sure it's disabled when you have a dumb terminal detected? We have code to do that already.

@devoncarew
Copy link
Member Author

Perhaps a welcome message the first time the tool is run, with a prominent message about analytics, and a 2 second delay before we continue with running the first command? Red text to me would signify an error condition; this is more something that we want to emphasize. I think the delay will help the user notice the message, instead of having it scroll quickly to the top of the screen.

@Hixie
Copy link
Contributor

Hixie commented May 3, 2016

It could be green text, or purple. :-)

I'm worried about a delay because it'll make us look slow.

You could always try \e[5m ... \e[25m.

@Hixie
Copy link
Contributor

Hixie commented May 3, 2016

\e[35m is magenta, FWIW.

@devoncarew
Copy link
Member Author

ok, updated:

  • made the usage text bold (on terminals that support it)
  • drew a double-bordered box around it
  • for commands like --help, have the text print after the main command text, so that it's not lost to scrolling
  • for other commands, print out after the engine artifacts are downloaded

screen shot 2016-05-03 at 2 55 14 pm

@devoncarew devoncarew merged commit 7138309 into flutter:master May 3, 2016
@Hixie
Copy link
Contributor

Hixie commented May 3, 2016

Well that looks awesome.

printStatus('');
printStatus('''
╔════════════════════════════════════════════════════════════════════════════════════════════════════╗
║ Welcome to Flutter! - Flutter version $versionString - https://flutter.io ║
Copy link
Contributor

Choose a reason for hiding this comment

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

getVersionString can return variable width strings, we should probably figure out the spacing here on the fly rather than hard-coding it. Feel free to file a bug on me if you want me to take care of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yeah; I was thinking that it was hard-coded to a specific length, but that's just for the first half of the version, not the branch name part.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 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.

None yet

2 participants