SamVerschueren / vscode-yo

Yeoman plugin for VS Code
MIT License
87 stars 15 forks source link

Show picker earlier with promise for progress #22

Closed bpasero closed 8 years ago

bpasero commented 8 years ago

I think it would be a nicer experience if the resolving of available yo-generators and the loading of a specific generator happens without the picker closing all the time and then reopen after a second or so.

You can always add a promise into the picker API and you will get progress while you resolve the things in the background.

SamVerschueren commented 8 years ago

Loading of the available generators is done with a promise but loading the questions indeed isn't if I'm not mistaken. Good suggestion!

SamVerschueren commented 8 years ago

It feels hacky but I don't think there is another way of doing it.

window.showQuickPick(new Promise((resolve, reject) => {
    setImmediate(() => {
        this._env.run(generator, this.done)
            .on('npmInstall', () => {
                this.setState('install node dependencies');
            })
            .on('bowerInstall', () => {
                this.setState('install bower dependencies');
            })
            .on('end', () => {
                this.clearState();
                console.log(`${EOL}${figures.tick} done`);
            });

        reject();
    });
})).then(undefined, () => { });

Maybe if there was a core method that allowed showing and hiding a progress bar?

progress

bpasero commented 8 years ago

@SamVerschueren your solution does not look hacky to me at all you are wrapping a promise around a long running operation. What you could maybe change is to promisify more of your code, e.g. this.env.run() should already return a promise.

SamVerschueren commented 8 years ago

this._env is a yeoman environment, can't do anything about it I'm afraid :).

The reason I think it feels hacky is because I'm showing a quickpick component which is never used, just in order to show the progress bar...

bpasero commented 8 years ago

Well, your entire UI is around the picker. When I start to use your extension I think it is fine to show progress via the same component that I am interacting with.

I would not like a solution where you would see some progress somewhere (e.g. status bar) where it reads "resolving available generators...", but that might be just me.

SamVerschueren commented 8 years ago

No, the user experience is way better now because the component is always visible. But I thought maybe there was another way of doing this. That's why I asked your opinion regarding the implementation :).

bpasero commented 8 years ago

I find it very natural to couple progress and promises together and not have an explicit way of showing progress. However, we do have some kind of progress service in the workbench that we can ask to show progress explicitly but this is currently not exposed to extensions. It would also not work very well in quick open currently. Thanks for fixing!

SamVerschueren commented 8 years ago

Reopening due to https://github.com/Microsoft/vscode/issues/693. Because the quick picker is not closed, the user will have a quick pick progress bar that just keeps spinning. That's why I reverted back this changes.

SamVerschueren commented 8 years ago

@bpasero just curious, will this be fixed in 0.11.0?

bpasero commented 8 years ago

@SamVerschueren yeah although the bug is still open, the picker will close properly and you should get the error handler.

SamVerschueren commented 8 years ago

Awesome!

Do I have to set my engines.vscode to 0.11.x when releasing this feature? Will users of VS Code 0.10.x still be able to download the previous version with engines.vscode set to 0.10.x? Couldn't find documentation regarding this.

bpasero commented 8 years ago

@SamVerschueren we are still discussing our version scheme, it will be well communicated once this happens!

SamVerschueren commented 8 years ago

:+1: