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.

Sign up for GitHub

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

Jump to bottom

Finalize notebookEditor API proposal #149271

Closed
Tracked by #149861
mjbvz opened this issue May 11, 2022 · 8 comments · Fixed by #149767
Closed
Tracked by #149861

Finalize notebookEditor API proposal #149271

mjbvz opened this issue May 11, 2022 · 8 comments · Fixed by #149767
Assignees
@rebornix @mjbvz
Labels
api-finalization insiders-released Patch has been released in VS Code Insiders notebook verification-needed Verification of issue is requested verified Verification succeeded
Milestone
May 2022

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented May 11, 2022

Splitting off from #125758

The notebookEditor API proposal looks to be in good shape. We may revise a few names or functions, but otherwise I think it is ready for testing and finalization
@mjbvz mjbvz added this to the May 2022 milestone May 11, 2022
@mjbvz mjbvz self-assigned this May 11, 2022
@mjbvz mjbvz changed the title Finalized notebookEditor API proposal Finalize notebookEditor API proposal May 11, 2022
@mjbvz mjbvz mentioned this issue May 11, 2022
Merged
@mjbvz
Copy link
Collaborator Author

mjbvz commented May 11, 2022

Pinging current consumers to see if you have feedback on the current state of the notebookEditor API

@rchiodo
Copy link
Contributor

rchiodo commented May 11, 2022

Works fine for us. I don't think we'd need any changes.

@joyceerhl
Copy link
Collaborator

No further feedback, RemoteHub's use of notebookEditor is due to a change from @roblourens to contribute the continue on action to notebook/cell/executePrimary. This should eventually not be necessary due to #141293

@tanhakabir
Copy link
Contributor

No feedback from me as well.

@alyssajotice
Copy link

No further feedback.

@dbaeumer
Copy link
Member

I am fine as well.

mjbvz added a commit to mjbvz/vscode that referenced this issue May 16, 2022
@mjbvz
Refine notebookEditor proposal
6acc212 
For microsoft#149271

- `NotebookEditor.document` -> `NotebookEditor.notebook`

- Add `selection` to for setting primary selections. Matches `TextDocument.selection`

- Deprecate `showNotebookDocument`
@mjbvz mjbvz mentioned this issue May 16, 2022
Merged
mjbvz added a commit that referenced this issue May 16, 2022
@mjbvz
Refine notebookEditor proposal (#149656)
cfbf3d5 
For #149271

- `NotebookEditor.document` -> `NotebookEditor.notebook`

- Add `selection` to for setting primary selections. Matches `TextDocument.selection`

- Deprecate `showNotebookDocument`
mjbvz added a commit to mjbvz/vscode that referenced this issue May 17, 2022
@mjbvz
Finalize NotebookEditor api proposal
eaa3cf0 
Fixes microsoft#149271

This finalizes most parts of the NotebookEditor api proposal. I haven't removed the proposal entirely as there are still a few parts being left behind:

- The deprecated properties/functions
- A few contribution points such as `notebook/cell/executePrimary`
@mjbvz mjbvz mentioned this issue May 17, 2022
Merged
@mjbvz
Copy link
Collaborator Author

mjbvz commented May 17, 2022

Thanks for the feedback! We believe the API is ready for finalization so I've submitted a PR that moves it over. A few minor changes for adopting it:

  • We've renamed NotebookEditor.document to NotebookEditor.notebook. The old property is kept for backcompat but marked deprecated

  • We've removed the showNotebookEditor overload that takes a Uri. Instead you can use openNotebookEditor + showNotebookEditor with a NotebookDocument

@mjbvz mjbvz mentioned this issue May 18, 2022
Closed
9 tasks
Mingpan pushed a commit to Mingpan/vscode that referenced this issue May 23, 2022
@mjbvz @Mingpan
Refine notebookEditor proposal (microsoft#149656)
453f640 
For microsoft#149271

- `NotebookEditor.document` -> `NotebookEditor.notebook`

- Add `selection` to for setting primary selections. Matches `TextDocument.selection`

- Deprecate `showNotebookDocument`
mjbvz added a commit that referenced this issue May 23, 2022
@mjbvz
Finalize NotebookEditor api proposal (#149767)
45304da 
* Finalize NotebookEditor api proposal

Fixes #149271

This finalizes most parts of the NotebookEditor api proposal. I haven't removed the proposal entirely as there are still a few parts being left behind:

- The deprecated properties/functions
- A few contribution points such as `notebook/cell/executePrimary`

* remove extra quote
@vscodenpa vscodenpa added the unreleased Patch has not yet been released in VS Code Insiders label May 23, 2022
mjbvz added a commit to mjbvz/vscode that referenced this issue May 23, 2022
@mjbvz
Finalize some interactive contribution points
da76894 
Follow up on microsoft#149271

Finalizing these two contributions based on microsoft#149767 (comment)

We're leaving the `notebook/cell/executePrimary` point for now as we don't plan to finalize it
@mjbvz mjbvz mentioned this issue May 23, 2022
Merged
mjbvz added a commit that referenced this issue May 23, 2022
@mjbvz
Finalize some interactive contribution points (#150234)
6a91081 
Follow up on #149271

Finalizing these two contributions based on #149767 (comment)

We're leaving the `notebook/cell/executePrimary` point for now as we don't plan to finalize it
@vscodenpa vscodenpa added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels May 24, 2022
@mjbvz mjbvz added the verification-needed Verification of issue is requested label May 28, 2022
@isidorn
Copy link
Contributor

isidorn commented Jun 7, 2022

I have opened this discussion on the extension author side to get feedback from the extension community
microsoft/vscode-discussions#27

@rebornix rebornix added the verified Verification succeeded label Jun 8, 2022
@mjbvz mjbvz mentioned this issue Jun 9, 2022
Closed
5 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@rebornix rebornix

@mjbvz mjbvz

Labels
api-finalization insiders-released Patch has been released in VS Code Insiders notebook verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Milestone
May 2022
Development

Successfully merging a pull request may close this issue.

Finalize NotebookEditor api proposal mjbvz/vscode
11 participants
@IanMatthewHuff @rebornix @isidorn @dbaeumer @tanhakabir @mjbvz @rchiodo @alyssajotice @joyceerhl @vscodenpa and others