Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

Make the default app generator prompt user for choice #67

Merged
merged 3 commits into from
Sep 16, 2019

Conversation

zone117x
Copy link
Collaborator

@zone117x zone117x commented Sep 10, 2019

Closes issue:


The default app generator now prompts user for choice:

npm -g yo generator-blockstack
yo blockstack

Results in:
image

Additionally, the yeoman-environment is now integrated into the package.json bin script. This now allows non-global usage and a single command.

Examples:

npx generator-blockstack
# ? Select Blockstack app output type (Use arrow keys)
# ❯ React 
#   Plain JavaScript 
#   Vue 
npx generator-blockstack --help
# Options:
#  -h,   --help           # Print the generator's options and usage
#        --skip-cache     # Do not remember prompt answers                     Default: false
#        --skip-install   # Do not automatically install dependencies          Default: false
#        --force-install  # Fail on install dependencies error                 Default: false
#        --react          # Generate a React app without prompting
#        --plain          # Generate a plain JavaScript app without prompting
#        --vue            # Generate a Vue app without prompting
npx generator-blockstack --react

# The above command is equivalent to running:
npm -g yo generator-blockstack
yo blockstack:react

Testing

The above commands referencing generator-blockstack won't work until the package is published to npm. This github branch must be fully specified. Example:

npx blockstack/blockstack-app-generator#feature/change-defaults
# or
npm install -g npx blockstack/blockstack-app-generator#feature/change-defaults

…ther app generators.

* Support for direct usage via npx.
@zone117x zone117x self-assigned this Sep 10, 2019
@zone117x zone117x added this to the DX Q3 Sprint 2 milestone Sep 10, 2019
@moxiegirl
Copy link
Contributor

What is the benefit of showing warnings when generating? We get quite a few which is not confidence-inspiring. For example:

5 warnings generated.
  CC(target) Release/obj.target/secp256k1/native/secp256k1/src/secp256k1.o
In file included from ../native/secp256k1/src/secp256k1.c:13:
../native/secp256k1/src/group_impl.h:686:12: warning: unused function 'secp256k1_gej_has_quad_y_var' [-Wunused-function]
static int secp256k1_gej_has_quad_y_var(const secp256k1_gej *a) {
           ^
../native/secp256k1/src/group_impl.h:113:13: warning: unused function 'secp256k1_ge_set_gej_var' [-Wunused-function]
static void secp256k1_ge_set_gej_var(secp256k1_ge *r, secp256k1_gej *a) {
            ^
../native/secp256k1/src/group_impl.h:267:12: warning: unused function 'secp256k1_gej_is_valid_var' [-Wunused-function]
static int secp256k1_gej_is_valid_var(const secp256k1_gej *a) {
           ^
In file included from ../native/secp256k1/src/secp256k1.c:15:
../native/secp256k1/src/ecmult_const_impl.h:123:13: warning: unused function 'secp256k1_ecmult_const' [-Wunused-function]
static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, const secp256k1_scalar *scalar) {
            ^
4 warnings generated.
  SOLINK_MODULE(target) Release/secp256k1.node

Could we suppress the warnings instead for better user experience? Many of our developers are not super experienced and those warnings can be intimidating.

app/index.js Outdated Show resolved Hide resolved
app/index.js Outdated Show resolved Hide resolved
app/index.js Show resolved Hide resolved
type: 'list',
name: 'generator_type',
message: 'Select Blockstack app output type',
choices: [
Copy link
Contributor

Choose a reason for hiding this comment

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

The (Use arrow keys) disappears after the one use. Is there any reason to make them disappear?

(Use arrow keys to select and press ENTER)

Many of our app mining users are beginner programmers from other countries. It helps to use complete sentences and instructions and to leave the instructions in place.

Copy link
Collaborator Author

@zone117x zone117x Sep 11, 2019

Choose a reason for hiding this comment

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

That is behavior and messaging hardcoded by what yo prompt uses: https://github.com/SBoudrias/Inquirer.js/blob/d3d632a7d94b3d719152c109f1865a8a9a2a2432/packages/inquirer/lib/prompts/list.js#L84-L86

One work around is to use raw index lists which let us customize that message, but then forces a number-based UI (arrow keys do still work):
image

Those appear to be our 2 options, I'm fine with either -- what do you think?

Copy link
Contributor

@moxiegirl moxiegirl Sep 11, 2019

Choose a reason for hiding this comment

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

Yeah, I suspected this might be the case. Um, I think the number based thing is a lot more obvious than the keyboard input. Let's go with that if you can do it easy. Thanks for adjusting this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated with that change ^

@zone117x
Copy link
Collaborator Author

@moxiegirl

What is the benefit of showing warnings when generating? We get quite a few which is not confidence-inspiring. For example:

5 warnings generated.
  CC(target) Release/obj.target/secp256k1/native/secp256k1/src/secp256k1.o
In file included from ../native/secp256k1/src/secp256k1.c:13:
../native/secp256k1/src/group_impl.h:686:12: warning: unused function 'secp256k1_gej_has_quad_y_var' [-Wunused-function]
static int secp256k1_gej_has_quad_y_var(const secp256k1_gej *a) {
           ^
../native/secp256k1/src/group_impl.h:113:13: warning: unused function 'secp256k1_ge_set_gej_var' [-Wunused-function]
static void secp256k1_ge_set_gej_var(secp256k1_ge *r, secp256k1_gej *a) {
            ^
../native/secp256k1/src/group_impl.h:267:12: warning: unused function 'secp256k1_gej_is_valid_var' [-Wunused-function]
static int secp256k1_gej_is_valid_var(const secp256k1_gej *a) {
           ^
In file included from ../native/secp256k1/src/secp256k1.c:15:
../native/secp256k1/src/ecmult_const_impl.h:123:13: warning: unused function 'secp256k1_ecmult_const' [-Wunused-function]
static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, const secp256k1_scalar *scalar) {
            ^
4 warnings generated.
  SOLINK_MODULE(target) Release/secp256k1.node

Could we suppress the warnings instead for better user experience? Many of our developers are not super experienced and those warnings can be intimidating.

Yeah I hate these warnings, they are in almost all of our projects as a result of bitcoinjs-lib dependencies. bitcoinjs-lib strictly follows a Don't trust. Verify. model, and refuse to bundle pre-compiled native binaries. I think they should be able to tweak their node-gyp build config so it doesn't output useless compiler warnings... I might make a PR to their stuff that attempts that.

Could we suppress the warnings instead for better user experience? Many of our developers are not super experienced and those warnings can be intimidating.

Hmm, possibly. However, yo already does a lot of stdin/stdout redirection and manipulation to provide the terminal UI, so if it is possible to hide these I think it would be pretty brittle and hacky. For example hiding all stderr output and causing legitimate errors to be then hidden. I think we should leave it be for this PR.

On a positive note -- the plans I have so far for secp256k1 related issues may do away with this dependency all together in our repos (progress on that will be captured in hirosystems/stacks.js#703).

@moxiegirl
Copy link
Contributor

moxiegirl commented Sep 11, 2019

@zone117x The prompting system should be tested on Windows and Ubuntu. I attempted some tests in Windows VM but hard to tell if the issues I encountered were related to Windows or the env I was running in. Specifically, arrow keys did not work to navigate the prompts. Though, arrow keys did work in general throughout my VM.

Aborting the interactive prompts resulted in this:

image

I did use ENTER to take the default React generation in my next test. The npm succeeded with odd errors:

image

image

image

Then, after the npm start to start the hello-blockstack app failed.

image

We should be testing this in Windows/Ubuntu as well as Mac OS, this generator should work in all three and produce a generated app the compiles in all three.

It could be the env in one or the other needs to be prettty specific to actually succeeded. I had limited time to do a full test run in Windows and none at all in Ubuntu.

cc: @timstackblock

@zone117x
Copy link
Collaborator Author

Looks like a problem with your environment. Installation and running works just fine for me with mingw, aside from the interactive rendering bugs. Those happen all the time with 3rd party bash TTY emulators for Windows. All yo generators with interactive prompts have the same rendering bugs. Just tested with the most popular and maintained yo package generator-jhipster and it has the same behavior.

Windows support is widely considered to be cmd.exe and Powershell, both of which work. Also tested with Ubuntu 18.04.

@moxiegirl
Copy link
Contributor

Thanks for testing. My env was definitely suspect, I was just using Git Bash.

Copy link
Contributor

@yknl yknl left a comment

Choose a reason for hiding this comment

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

Works great for me 👍

@yknl yknl modified the milestones: DX Q3 Sprint 2, DX Q3 Sprint 3 Sep 16, 2019
@zone117x zone117x merged commit 2ca4cea into master Sep 16, 2019
moxiegirl pushed a commit to moxiegirl/docs.blockstack that referenced this pull request Sep 26, 2019
Signed-off-by: Mary Anthony <mary@blockstack.com>
moxiegirl pushed a commit to stacks-network/docs that referenced this pull request Sep 26, 2019
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

3 participants