Skip to content

Fix notebook preview#1003

Draft
seeM wants to merge 20 commits into
mainfrom
fix/notebook-preview
Draft

Fix notebook preview#1003
seeM wants to merge 20 commits into
mainfrom
fix/notebook-preview

Conversation

@seeM

@seeM seeM commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@seeM seeM requested a review from vezwork June 12, 2026 16:46
@posit-snyk-bot

posit-snyk-bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@seeM seeM force-pushed the fix/notebook-preview branch from af78296 to d8d1730 Compare June 12, 2026 17:42
Comment on lines +56 to 62
function isNotebook(doc?: vscode.TextDocument | vscode.NotebookDocument): doc is vscode.NotebookDocument {
return !!doc && 'notebookType' in doc;
}

export function isNotebookUri(uri: Uri) {
function isIpynbUri(uri: Uri) {
return extname(uri.fsPath).toLowerCase() === ".ipynb";
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Nit: prefer dog !== undefined over !!doc for messiness of truthiness reasons

  2. Could you add a comment explaining why 'notebookType' in doc is the thing to check for isNotebook? Is notebook that same as ipynb? If we're changing isNotebookUri to isIpynbUri, should we also change isNotebook to isIpynb?

const frontMatter = targetEditor.notebook
? targetEditor.notebook.cellAt(0)?.document.getText() || ""
: documentFrontMatterYaml(this.engine_, targetEditor.document);
const frontMatter = targetEditor.frontMatterYaml(this.engine_);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is there less cases than before? It doesn't look like targetEditor.frontMatterYaml in editor.ts captures all these cases?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants