Allow promise for locals function in blueprints #4025
Conversation
|
Probably something to do with readline, maybe here: https://github.com/ember-cli/ember-cli/blob/master/lib/ui/index.js#L126 |
|
@MajorBreakfast you contributed to that code, do you have any thoughts about this issue? |
|
I knew about this bug, but didn't investigate further at the time. My solution was to use one of the other input styles. |
|
Hum, maybe @SBoudrias has some thoughts. |
|
In Inquirer, we mute the input with mute-stream. Maybe something is overridding it? |
|
@stefanpenner do you have any ideas about this issue? |
unfortunately nothing concrete. It would seem like we need a more complex UI stack here. Basically, when we move to inquirer, we likely need to "end" the previous UI session, then "resume" it once the inquirer step has completed. I suspect this would also make a good refactoring. Unfortunately, short of this, I am unaware of another approach. This of course does not mean that this isn't possible, so further investigation likely is a good idea. |
|
@stefanpenner where is the "previous UI session"? |
|
@knownasilya what's the state of this PR? anyone still working on it? is it still relevant? |
|
I think it's still relevant, and by itself it's pretty much finished. It's the other pieces to the puzzle that need to be figured out. |
|
This should be fixed by my PR #5621. |
knownasilya
force-pushed
the
locals-promise
branch
from
6238de0
to
253d4f2
Compare
|
A little stumped why the other tests are failing, the result from |
|
#5621 is scheduled for merge |
| }; | ||
|
|
||
| return merge({}, standardLocals, customLocals); | ||
| return Promise.resolve(this.locals(options)) |
There was a problem hiding this comment.
lets use the constructor form here, in-case locals throws.
return new Promise(function(resolve) {
resolve(this.locals(options);
}.bind(this))This ensures the function returns a promise correctly, and this.locals behaves like other functions who's return value may be a promise.
|
left 1 comment, once addressed we can merge. |
|
@stefanpenner change made, but I still don't know why the tests are broken. |
|
@homu r+ |
|
📌 Commit a8a9465 has been approved by |
Allow promise for locals function in blueprints This is a WIP for moving ember-cli/rfcs#7 forward. Basically you can test it with `ember new [name] --blueprint https://github.com/knownasilya/app-blueprint.git` The issue is that you cannot select inquirer options using your keys. Basically this is the output:  "Leaflet" is the default here, and hitting up/down creates those characters below.
|
☀️ Test successful - status |
This is a WIP for moving ember-cli/rfcs#7 forward.
Basically you can test it with
ember new [name] --blueprint https://github.com/knownasilya/app-blueprint.gitThe issue is that you cannot select inquirer options using your keys. Basically this is the output:
"Leaflet" is the default here, and hitting up/down creates those characters below.