Add notebook cell support for isort by edvilme · Pull Request #565 · microsoft/vscode-isort

edvilme · 2026-03-03T22:36:02Z

Summary

With the upgrade to pygls 2, the extension can now leverage LSP 3.17's notebook document sync protocol. This PR adds Jupyter notebook cell support to the isort extension, enabling import sorting diagnostics and code actions for Python cells within notebooks in both server and server-less modes. Closes #303.

Also removes the is_python guard that used ast.parse to validate Python code before running isort. This check rejected valid-but-incomplete Python code, preventing organize imports from working on files being actively edited. Closes #243.

Changes

LSP Server (`bundled/tool/lsp_server.py`)

Notebook document sync: Declared NotebookDocumentSyncOptions for jupyter-notebook and interactive notebook types with Python cell filtering
Notebook handlers: Added notebookDocument/didOpen, didChange, didSave, and didClose handlers that run diagnostics on notebook cells
Cell filtering: Non-code cells (e.g., Markdown) are filtered out using NotebookCellKind.Code checks
Cell structure changes: The didChange handler processes text content edits, cell additions (structure.did_open), and cell removals (structure.did_close)
Removed is_python guard: Removed is_python, strip_magic_commands, and MAGIC_COMMAND_REGEX — isort handles magic commands and syntax errors gracefully on its own, and the ast.parse check was incorrectly rejecting incomplete code ("no organize imports action available" error on uncompleted code #243)
Docstring fix: Corrected _get_document_path example to show .ipynb output instead of .py

Server-less mode (`src/common/runner.ts`, `src/common/sortImports.ts`)

Notebook URI resolution: Added getDocumentPath helper to runner.ts that resolves vscode-notebook-cell:// URIs to filesystem paths, mirroring _get_document_path on the Python side
Full notebook support: Removed the isNotebookCell guard — server-less mode now fully supports notebook cells
URI handling: Changed code action data from uri.fsPath to uri.toString() so notebook cell URIs round-trip correctly through code action resolution

Tests

Session client: Added notebook notification methods (notify_notebook_did_open/did_change/did_save/did_close) to the LSP test client
New test file (test_notebook.py): 5 tests covering all notebook handlers:
- test_notebook_did_open — diagnostics published on notebook open
- test_notebook_did_change_text_content — diagnostics update on cell edit
- test_notebook_did_change_structure_add_cell — new cells are linted
- test_notebook_did_save — all cells re-linted on save
- test_notebook_did_close — diagnostics cleared on close
Incomplete code tests: Added tests for both regular documents and notebook cells with syntactically incomplete Python code to verify isort works without the is_python guard
Mock updates: Added notebook-related types to the lsprotocol mock in test_get_cwd.py to fix CI import failures

- Add notebook document sync options for jupyter-notebook and interactive - Add LSP handlers for notebook did_open, did_change, did_save, did_close - Remove notebook cell guard in server-less code action provider - Use uri.toString() instead of fsPath for code action data

- Add missing PEP 8 blank lines between top-level functions in lsp_server.py - Restore notebook cell guard in server-less mode code action provider - Revert unrelated package-lock.json peer changes - Fix prettier formatting (catch {}, empty arrow functions)

Lint newly added cells and clear diagnostics for removed cells, not just cells with text content changes.

The function resolves notebook cell URIs to the notebook's .ipynb path (stripping the cell fragment), not to a .py path.

- Add notebook notification methods to LspSession test client - Add tests for notebookDocument/didOpen, didChange, didSave, didClose - Test cell structure changes (adding new cells with unsorted imports) - Test diagnostics are cleared on notebook close

The mock lsprotocol.types module was missing notebook-related constants and types needed by the module-level NOTEBOOK_SYNC_OPTIONS in lsp_server.py, causing import failures in CI.

edvilme · 2026-03-03T22:53:58Z

Fixes #303

src/test/python_tests/test_notebook.py

src/common/sortImports.ts

bundled/tool/lsp_server.py

Use cell.kind check to skip markdown cells, matching the pattern used in notebook_did_save.

Consistent with the other .fsPath to .toString() changes in this file — fsPath loses scheme information for non-file URIs.

Add getDocumentPath helper to runner.ts that resolves notebook cell URIs to filesystem paths (mirroring _get_document_path in Python). Remove the isNotebookCell guard in sortImports.ts since the runner can now handle notebook URIs.

The notebook document URI should use the file: scheme (it represents the .ipynb file on disk). Only individual cell URIs use the vscode-notebook-cell: scheme.

rchiodo

Approved via Review Center.

Remove is_python, strip_magic_commands, and MAGIC_COMMAND_REGEX from lsp_server.py. The is_python check used ast.parse which rejected valid-but-incomplete Python code, preventing organize imports from working on files being actively edited. Closes microsoft#243 Also removes now-unused ast and re imports. Adds tests for both regular documents and notebook cells with incomplete Python code (missing function body) to verify isort correctly sorts imports without the is_python guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

edvilme added 5 commits March 3, 2026 14:11

Handle cell structure changes in notebook_did_change

1e7aedc

Lint newly added cells and clear diagnostics for removed cells, not just cells with text content changes.

Fix _get_document_path docstring example

c670c96

The function resolves notebook cell URIs to the notebook's .ipynb path (stripping the cell fragment), not to a .py path.

edvilme marked this pull request as draft March 3, 2026 22:37

edvilme added 3 commits March 3, 2026 14:46

Add notebook types to lsprotocol mock in test_get_cwd

afb55ee

The mock lsprotocol.types module was missing notebook-related constants and types needed by the module-level NOTEBOOK_SYNC_OPTIONS in lsp_server.py, causing import failures in CI.

Run black on lsp_server.py

54fb106

Run black on test_notebook.py

f69cfa4

edvilme added the feature-request Request for new features or functionality label Mar 3, 2026

edvilme marked this pull request as ready for review March 3, 2026 22:54

edvilme linked an issue Mar 3, 2026 that may be closed by this pull request

No Organize Imports or Python Refactor: Sort Imports options available in Jupyter Notebooks #303

Closed

rchiodo reviewed Mar 3, 2026

View reviewed changes

src/test/python_tests/test_notebook.py Show resolved Hide resolved

rchiodo reviewed Mar 3, 2026

View reviewed changes

src/common/sortImports.ts Outdated Show resolved Hide resolved

rchiodo reviewed Mar 3, 2026

View reviewed changes

bundled/tool/lsp_server.py Show resolved Hide resolved

edvilme added 4 commits March 3, 2026 15:20

Filter out non-code cells in notebook_did_open

040b397

Use cell.kind check to skip markdown cells, matching the pattern used in notebook_did_save.

Use uri.toString() in notebook cell warning log

a8f3f9c

Consistent with the other .fsPath to .toString() changes in this file — fsPath loses scheme information for non-file URIs.

Support notebook cells in server-less mode

3ee8e76

Add getDocumentPath helper to runner.ts that resolves notebook cell URIs to filesystem paths (mirroring _get_document_path in Python). Remove the isNotebookCell guard in sortImports.ts since the runner can now handle notebook URIs.

Fix _make_notebook_uri to use file: scheme

3a13bd2

The notebook document URI should use the file: scheme (it represents the .ipynb file on disk). Only individual cell URIs use the vscode-notebook-cell: scheme.

edvilme requested a review from rchiodo March 3, 2026 23:40

rchiodo previously approved these changes Mar 3, 2026

View reviewed changes

edvilme dismissed rchiodo’s stale review via d7e1d4a March 4, 2026 00:02

edvilme requested a review from rchiodo March 4, 2026 00:05

edvilme enabled auto-merge (squash) March 4, 2026 00:06

rchiodo approved these changes Mar 4, 2026

View reviewed changes

edvilme merged commit 6a673e8 into microsoft:main Mar 4, 2026
19 checks passed

Copilot AI mentioned this pull request Mar 4, 2026

Add Jupyter notebook cell support via LSP 3.17 Notebook Document Sync microsoft/vscode-python-tools-extension-template#265

Open

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add notebook cell support for isort#565

Add notebook cell support for isort#565
edvilme merged 13 commits intomicrosoft:mainfrom
edvilme:notebook-cells

edvilme commented Mar 3, 2026 •

edited

Loading

Uh oh!

edvilme commented Mar 3, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rchiodo left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

edvilme commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

LSP Server (bundled/tool/lsp_server.py)

Server-less mode (src/common/runner.ts, src/common/sortImports.ts)

Tests

Uh oh!

edvilme commented Mar 3, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rchiodo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

edvilme commented Mar 3, 2026 •

edited

Loading

LSP Server (`bundled/tool/lsp_server.py`)

Server-less mode (`src/common/runner.ts`, `src/common/sortImports.ts`)