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

Create Python Environment: show a back button and ability to detect user cancellation #20274

Closed
DonJayamanne opened this issue Nov 24, 2022 · 16 comments · Fixed by #20693
Closed
Assignees
Labels
area-environments Features relating to handling interpreter environments author-verification-requested Issues potentially verifiable by issue author feature-request Request for new features or functionality needs PR Ready to be worked on partner ask verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@DonJayamanne
Copy link

DonJayamanne commented Nov 24, 2022

Is it possible to have a back button on the quick pick displayed for env creation.
This way when users go into kernel picker and choose the ability to create a kernel, then they can go back.

Else today when users hit the cancel button there's no way to know the user hit cancel

Suggestions:'

  • When command is called from an extension add support for an argument which would indicate whether a back button is required
  • If this new argument is prohibited an user hits the back button then throw a CancellationError (vscode error to notify of the cancellation)
  • If the user hits cancel and dismisses the quick pick , then do as previous step
@DonJayamanne DonJayamanne added the feature-request Request for new features or functionality label Nov 24, 2022
@github-actions github-actions bot added the triage-needed Needs assignment to the proper sub-team label Nov 24, 2022
@brettcannon brettcannon added partner ask needs PR Ready to be worked on area-environments Features relating to handling interpreter environments and removed triage-needed Needs assignment to the proper sub-team labels Nov 24, 2022
@DonJayamanne
Copy link
Author

DonJayamanne commented Jan 10, 2023

@brettcannon @karrtikr
Are you happy for me to submit a PR for this.
Would you prefer a new API method or re-use the same command?

@karrtikr
Copy link

karrtikr commented Jan 11, 2023

@karthiknadig owns this component. What do you think?

@brettcannon
Copy link
Member

This way when users go into kernel picker and choose the ability to create a kernel, then they can go back.

Else today when users hit the cancel button there's no way to know the user hit cancel

So is the concern more that you are somewhere part way in picking a kernel and you don't know someone dropped out of it? I can see a back button happening when someone realizes they made a mistake, but that won't solve knowing about a cancellation in all cases, correct?

@karthiknadig
Copy link
Member

I would like some clarity on the flow. It is a confusing on how back button will be used here.

@DonJayamanne
Copy link
Author

DonJayamanne commented Jan 11, 2023

@karthiknadig
Please could you install the latest pre-release of Jupyter and Python extension in vsocode insiders

  • Then add the following setting into your user settings file
"notebook.kernelPicker.type": "mru",
  • Open a new empty workspace folder
  • Create a new ipynb file
  • Go into the kernel picker,. and select Python Environment and use the option to create a new python env
    That will take you to the Python Quick pick to create a Python env (venv or conda), however one you get there
    there's no way to cancel that and go back to the original kernel picker UI
    We would like the whole workflow to be a smooth process so that users have the ability to go back (e.g. they might not want to create a venv or the like).

@brettcannon
Copy link
Member

  • Then add the following setting into your user settings file

Which settings are you referring to?

@DonJayamanne
Copy link
Author

Oops, updated

@DonJayamanne
Copy link
Author

@karthiknadig do you have an update on this?

@karthiknadig
Copy link
Member

@DonJayamanne This won't make it in the current iteration. We have two public APIs that we need to update with this change. Since we will be exposing this to other extensions we want to make sure that the back support is generic enough to work for the scenarios intended with the API.

@karthiknadig
Copy link
Member

@karrtikr We will need to discuss changes to environment selector and its API.

@DonJayamanne
Copy link
Author

DonJayamanne commented Feb 2, 2023

@brettcannon @karthiknadig

Please could we also get the following feature:

  • Notify caller when user hits the cancel button during any stage of the Creationg (e.g. esc key in the quick pick)

This is requires so that we know the user cancelled the env creation, else we do not know this and could end up re-displaying the same quick pick or other quick picks in the Jupyter extension

Throwing the vscode.Cancellation error would work (as this would indicate that the flow has been cancelled by the user)
Note: This is the same error VS Code uses for such workflow related tasks.

@DonJayamanne DonJayamanne changed the title Create Python Environment: show a back button Create Python Environment: show a back button and ability to detect uyser cancellation Feb 2, 2023
@DonJayamanne DonJayamanne changed the title Create Python Environment: show a back button and ability to detect uyser cancellation Create Python Environment: show a back button and ability to detect user cancellation Feb 2, 2023
@DonJayamanne
Copy link
Author

@karthiknadig Just wanted to check if there was any update on this or whether you could provide some guidance on what this API would look like. thanks

@karthiknadig
Copy link
Member

The command for this will have an optional field that takes showBackButton, with that field set, when user hits back or cancel (keyboard escape) we will raise an exception indicating if it was Back or Cancel.

@DonJayamanne
Copy link
Author

Thanks, however we will need to differentiate between cancel and back
As cancel means dismiss the whole workflow
And back means go back to the previous quickpick

@karthiknadig
Copy link
Member

Yes, "Back" and "Cancel" represent distinct user actions and therefore the API will represent them as separate exceptions.

karthiknadig added a commit that referenced this issue Feb 14, 2023
Closes #20274


### Usage

This change allows callers of the Create Environment command to handle
`Back` and `Cancel`:
``` typescript
let result: CreateEnvironmentResult | undefined;
try {
    const result = await commands.executeCommand("python.createEnvironment", {showBackButton: true});
} catch(e) {
   // error while creating environment
}

if (result?.action === 'Back') {
    // user clicked Back
}

if (result?.action === 'Cancel') {
    // user pressed escape or Cancel
}
```
I decided to go with `result?.action` because we don't have a npm
package for python extension API so catching particular exception might
be error prone with `ex instanceof <error>`. We will provide a proper
interface via `api.environments` for create environment, and
contribution to create environment. Until that point this command will
provide the stop gap.

### Notes

1. I did not use the multi-step input that is used in the rest of the
extension because, the existing implementation does not have context.
Consider the following scenario: venv -> workspace select -> python
select -> packages. Assume that there is only one workspace, and we
don't show the workspace selection UI, that decision is done inside the
workspace step. So, if there is only 1 workspace it is a short circuit
to next step. User is on python selection and clicks `back`, workspace
selection short circuits to next step which is python selection. So,
from user perspective, back does not work. This can be fixed by sending
context that the reason control moved to previous step was because user
clicked on back.
2. This makes a change to old multi step API to rethrow the exception,
if user hits `back` and the current step has no steps to go back to.
@karthiknadig
Copy link
Member

karthiknadig commented Feb 14, 2023

Minor change to the above description, the action for the command will be returned using a field. We will provide a proper extension API for this, the solution below is a stop gap till then. Below is how you can get the UI to show back button, and action field to see what action was taken.

let result: CreateEnvironmentResult | undefined;
try {
    const result = await commands.executeCommand("python.createEnvironment", {showBackButton: true});
} catch(e) {
   // error while creating environment
}

if (result?.action === 'Back') {
    // user clicked Back
}

if (result?.action === 'Cancel') {
    // user pressed escape or Cancel
}

@karthiknadig karthiknadig added this to the February 2023 milestone Feb 21, 2023
@karthiknadig karthiknadig added verification-needed Verification of issue is requested author-verification-requested Issues potentially verifiable by issue author labels Feb 21, 2023
@joyceerhl joyceerhl added the verified Verification succeeded label Feb 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 25, 2023
wesm pushed a commit to posit-dev/positron that referenced this issue Mar 28, 2024
--------------------
Commit message for microsoft/vscode-python@fc72be9:

Show `Python: Report issue` command in palette regardless of whether a Python file is opened (microsoft/vscode-python#20726)

Closes microsoft/vscode-python#20723
--------------------
Commit message for microsoft/vscode-python@c18e8c9:

Detect ActiveState Python runtimes (microsoft/vscode-python#20534)

Closes microsoft/vscode-python#20532
--------------------
Commit message for microsoft/vscode-python@2152cd9:

Don't set `formatOnType` for auto-indent experiment if it's already set (microsoft/vscode-python#20710)


--------------------
Commit message for microsoft/vscode-python@995b0bc:

Add support for 'back' to all create env UI. (microsoft/vscode-python#20693)

Closes microsoft/vscode-python#20274


### Usage

This change allows callers of the Create Environment command to handle
`Back` and `Cancel`:
``` typescript
let result: CreateEnvironmentResult | undefined;
try {
    const result = await commands.executeCommand("python.createEnvironment", {showBackButton: true});
} catch(e) {
   // error while creating environment
}

if (result?.action === 'Back') {
    // user clicked Back
}

if (result?.action === 'Cancel') {
    // user pressed escape or Cancel
}
```
I decided to go with `result?.action` because we don't have a npm
package for python extension API so catching particular exception might
be error prone with `ex instanceof <error>`. We will provide a proper
interface via `api.environments` for create environment, and
contribution to create environment. Until that point this command will
provide the stop gap.

### Notes

1. I did not use the multi-step input that is used in the rest of the
extension because, the existing implementation does not have context.
Consider the following scenario: venv -> workspace select -> python
select -> packages. Assume that there is only one workspace, and we
don't show the workspace selection UI, that decision is done inside the
workspace step. So, if there is only 1 workspace it is a short circuit
to next step. User is on python selection and clicks `back`, workspace
selection short circuits to next step which is python selection. So,
from user perspective, back does not work. This can be fixed by sending
context that the reason control moved to previous step was because user
clicked on back.
2. This makes a change to old multi step API to rethrow the exception,
if user hits `back` and the current step has no steps to go back to.
--------------------
Commit message for microsoft/vscode-python@f3ecbf5:

Fix-conda-version-parsing (microsoft/vscode-python#20674)


--------------------
Commit message for microsoft/vscode-python@a6a6f50:

Inactive pytest run command (microsoft/vscode-python#20653)

Here the new flow is created but kept inactive for the pytest execution
--------------------
Commit message for microsoft/vscode-python@2202fbe:

Call the correct API to determine if a user is in treatment or control group (microsoft/vscode-python#20690)

Closes microsoft/vscode-python#20183
--------------------
Commit message for microsoft/vscode-python@b0ab10d:

Only use activated environment from terminal if VSCode was launched via CLI (microsoft/vscode-python#20667)

Closes microsoft/vscode-python#20644
--------------------
Commit message for microsoft/vscode-python@02a92fc:

Ensure interpreter path isn't truncated for workspace-relative paths when storing value (microsoft/vscode-python#20661)

For microsoft/vscode-python#20660

I'm not quite sure why this was done. It doesn't make sense to do this
only for display.
--------------------
Commit message for microsoft/vscode-python@377067f:

Use correct API to get interpreter path for language servers (microsoft/vscode-python#20656)

For microsoft/vscode-python#20644 closes
microsoft/vscode-python#20657
--------------------
Commit message for microsoft/vscode-python@cd6ca9d:

Remove `isort` extension dependency (microsoft/vscode-python#20577)

Closes microsoft/vscode-python#20586

Lead-authored-by: Kartik Raj <[email protected]>
Co-authored-by: Erik De Bonte <[email protected]>
Co-authored-by: Karthik Nadig <[email protected]>
Co-authored-by: mitchell <[email protected]>
Co-authored-by: Pete Farland <[email protected]>
Co-authored-by: Eleanor Boyd <[email protected]>
wesm pushed a commit to posit-dev/positron that referenced this issue Mar 28, 2024
--------------------
Commit message for microsoft/vscode-python@fc72be9:

Show `Python: Report issue` command in palette regardless of whether a Python file is opened (microsoft/vscode-python#20726)

Closes microsoft/vscode-python#20723
--------------------
Commit message for microsoft/vscode-python@c18e8c9:

Detect ActiveState Python runtimes (microsoft/vscode-python#20534)

Closes microsoft/vscode-python#20532
--------------------
Commit message for microsoft/vscode-python@2152cd9:

Don't set `formatOnType` for auto-indent experiment if it's already set (microsoft/vscode-python#20710)


--------------------
Commit message for microsoft/vscode-python@995b0bc:

Add support for 'back' to all create env UI. (microsoft/vscode-python#20693)

Closes microsoft/vscode-python#20274


### Usage

This change allows callers of the Create Environment command to handle
`Back` and `Cancel`:
``` typescript
let result: CreateEnvironmentResult | undefined;
try {
    const result = await commands.executeCommand("python.createEnvironment", {showBackButton: true});
} catch(e) {
   // error while creating environment
}

if (result?.action === 'Back') {
    // user clicked Back
}

if (result?.action === 'Cancel') {
    // user pressed escape or Cancel
}
```
I decided to go with `result?.action` because we don't have a npm
package for python extension API so catching particular exception might
be error prone with `ex instanceof <error>`. We will provide a proper
interface via `api.environments` for create environment, and
contribution to create environment. Until that point this command will
provide the stop gap.

### Notes

1. I did not use the multi-step input that is used in the rest of the
extension because, the existing implementation does not have context.
Consider the following scenario: venv -> workspace select -> python
select -> packages. Assume that there is only one workspace, and we
don't show the workspace selection UI, that decision is done inside the
workspace step. So, if there is only 1 workspace it is a short circuit
to next step. User is on python selection and clicks `back`, workspace
selection short circuits to next step which is python selection. So,
from user perspective, back does not work. This can be fixed by sending
context that the reason control moved to previous step was because user
clicked on back.
2. This makes a change to old multi step API to rethrow the exception,
if user hits `back` and the current step has no steps to go back to.
--------------------
Commit message for microsoft/vscode-python@f3ecbf5:

Fix-conda-version-parsing (microsoft/vscode-python#20674)


--------------------
Commit message for microsoft/vscode-python@a6a6f50:

Inactive pytest run command (microsoft/vscode-python#20653)

Here the new flow is created but kept inactive for the pytest execution
--------------------
Commit message for microsoft/vscode-python@2202fbe:

Call the correct API to determine if a user is in treatment or control group (microsoft/vscode-python#20690)

Closes microsoft/vscode-python#20183
--------------------
Commit message for microsoft/vscode-python@b0ab10d:

Only use activated environment from terminal if VSCode was launched via CLI (microsoft/vscode-python#20667)

Closes microsoft/vscode-python#20644
--------------------
Commit message for microsoft/vscode-python@02a92fc:

Ensure interpreter path isn't truncated for workspace-relative paths when storing value (microsoft/vscode-python#20661)

For microsoft/vscode-python#20660

I'm not quite sure why this was done. It doesn't make sense to do this
only for display.
--------------------
Commit message for microsoft/vscode-python@377067f:

Use correct API to get interpreter path for language servers (microsoft/vscode-python#20656)

For microsoft/vscode-python#20644 closes
microsoft/vscode-python#20657
--------------------
Commit message for microsoft/vscode-python@cd6ca9d:

Remove `isort` extension dependency (microsoft/vscode-python#20577)

Closes microsoft/vscode-python#20586

Lead-authored-by: Kartik Raj <[email protected]>
Co-authored-by: Erik De Bonte <[email protected]>
Co-authored-by: Karthik Nadig <[email protected]>
Co-authored-by: mitchell <[email protected]>
Co-authored-by: Pete Farland <[email protected]>
Co-authored-by: Eleanor Boyd <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-environments Features relating to handling interpreter environments author-verification-requested Issues potentially verifiable by issue author feature-request Request for new features or functionality needs PR Ready to be worked on partner ask verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants