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

Test: Notebook & Cell level CodeActions #183855

Closed
2 tasks done
Yoyokrazy opened this issue May 30, 2023 · 3 comments
Closed
2 tasks done

Test: Notebook & Cell level CodeActions #183855

Yoyokrazy opened this issue May 30, 2023 · 3 comments

Comments

@Yoyokrazy
Copy link
Contributor

Yoyokrazy commented May 30, 2023

Refs: #179213

Complexity: 3


Summary:

Notebooks now support CodeActions being run upon save. One extra CodeActionKind has been introduced for CodeActions that apply to the entire scope of the notebook, editing multiple cells (i.e., extracting imports to a top cell, grouping %pip install's, etc.). Source level CodeActions are also supported on an individual cell level. These run in parallel so they are restricted to only editing the resource that is passed to it (the TextDocument of the cell the provider is called upon).

For Notebook Level CodeActions, the provider is only passed the FIRST cell of the notebook, as to not return CodeActions that need to resolve for each cell. Keep this in mind if you choose to create your own provider/action.

Steps to Test:

**note: testing Notebook level CodeActions will require OSS, or tomorrow's (05/31) insiders build

  • Download or create your own extension, creating a provider for both cell/source and Notebook CodeActions
    • This is a sample extension that has the two providers
  • Launch the extension host
  • Create a new or open a pre-existing notebook
    • This is a playground I use with a couple messy notebooks that can be tested on.
  • Go to Settings (UI) and search for notebook.codeActionsOnSave, and add your CodeActions.
    • If you use the sample extension, your settings.json should include something like this to test both Cell and Notebook levels:
"notebook.codeActionsOnSave": {
	"notebook.cleanImports": true,
	"source.fixAll": true,
},
  • Save the document, triggering the CodeActions. May be easier to ensure they work individually by commenting one or the other CodeAction within your settings, then testing them combined at the end.

Things to check

  • CodeActions are applied correctly, the changes desired are applied without error.
  • The notebook is clean and saved after all CodeActions are applied.

If you want to do more

  • Create a source level CodeAction that applies edit to multiple cells. This should FAIL and not apply, due to the restriction of only editing the given cell/resource

Thanks so much for testing!

@Yoyokrazy Yoyokrazy added this to the May 2023 milestone May 30, 2023
@bhavyaus bhavyaus self-assigned this May 30, 2023
@sandy081 sandy081 assigned jrieken and ulugbekna and unassigned bhavyaus May 31, 2023
@ulugbekna
Copy link
Contributor

One extra CodeActionKind has been introduced for CodeActions that apply to the entire scope of the notebook

I can't seem to find this new code action kind in vscode.d.ts. The clean-nb-imports-ext repo has kind notebook.cleanImports but in a non-typesafe way (using a private constructor, that is). I also couldn't find it in proposed API.

Code actions work as described, however.

@ulugbekna ulugbekna removed their assignment May 31, 2023
jrieken added a commit to microsoft/vscode-github-issue-notebooks that referenced this issue May 31, 2023
@jrieken
Copy link
Member

jrieken commented May 31, 2023

@Yoyokrazy Using fa8eefda18bf099d82b58453f8fe5d3010196fa0 but I don't really get this to work. For GH issue notebooks I have added a code action provider that upper-cases all variables names but I am not being called on save. But for each cell which I actually wasn't expecting given the notebook.source.normalizeVariableNames kind.

You can check my provider here: https://github.com/microsoft/vscode-github-issue-notebooks/blob/2199dd30d50f1d90688df105a10f86f1cf55b513/src/extension/languageProvider.ts#L566

My config has this

  "notebook.codeActionsOnSave": {
    "notebook.source.normalizeVariableNames": true
  }

@Yoyokrazy
Copy link
Contributor Author

but I am not being called on save

@jrieken Did some troubleshooting here, and this is due to the design of the CodeAction SaveParticipant. It's only calling the provider for the first cell of the notebook, to ensure that we aren't running these against every cell. The first cell in the notebook that is in that repo was a markdown cell, which wouldn't connect with the selector you used to register your provider.

Once the cells were switched such that a code cell was cell index [0], all of the CodeActions were applied correctly, however the edit being applied wasn't working correctly given your intended functionality. I suspect that this is an issue with the implementation of the provider, as the returned CodeAction still only touched a single cell with its edit.

To troubleshoot a bit more, I also removed the "notebook." portion of notebook.source.normalizeVariableNames to test its behavior as a source action applied to every cell in parallel, and correctly ran into the check preventing cell/source actions from editing multiple resources. This confirms that restriction is working properly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants