diff --git a/src/vs/base/browser/positron/renderHtml.tsx b/src/vs/base/browser/positron/renderHtml.tsx index 6a04ce10d843..ecbde4f318b2 100644 --- a/src/vs/base/browser/positron/renderHtml.tsx +++ b/src/vs/base/browser/positron/renderHtml.tsx @@ -69,11 +69,12 @@ export const renderHtml = (html: string, opts: HTMLRendererOptions = {}): React. if (node.type === 'text') { // Create elements to host the text content. - if (node.content && node.content.trim().length > 0) { + // Preserve all text content, including whitespace-only nodes, as they may be + // semantically significant (e.g., spaces between inline elements). + if (node.content && node.content.length > 0) { return React.createElement('span', {}, node.content); } - // Text nodes with no content (or only whitespae content) are - // currently ignored. + // Only ignore truly empty text nodes (null, undefined, or empty string). return undefined; } else if (node.type === 'tag' && node.children) { if (node.children.length === 1 && node.children[0].type === 'text') { diff --git a/src/vs/base/common/htmlParser.ts b/src/vs/base/common/htmlParser.ts index eacdb65deb39..138436afa179 100644 --- a/src/vs/base/common/htmlParser.ts +++ b/src/vs/base/common/htmlParser.ts @@ -84,6 +84,29 @@ const voidElements: Record = { 'wbr': true }; +/** + * Checks if a node is nested inside a
 element.
+ * In 
 elements, whitespace must be preserved per HTML spec.
+ *
+ * This is an O(n) operation, where n is the depth of the node in the tree which
+ * is not super ideal and could be optimized with additional state tracking if
+ * needed but until we see performance issues the encapsulation provided by
+ * this implementation is worth it.
+ *
+ * @param node The node to check (typically parent of a text node)
+ * @returns true if node is inside a 
 element
+ */
+function isInsidePreElement(node: HtmlNode | undefined): boolean {
+	let current = node;
+	while (current) {
+		if (current.name === 'pre') {
+			return true;
+		}
+		current = current.parent;
+	}
+	return false;
+}
+
 /**
  * Parses a single HTML tag.
  *
@@ -355,7 +378,8 @@ export function parseHtml(html: string): Array {
 
 				// If a node is nothing but whitespace, collapse it as the spec states:
 				// https://www.w3.org/TR/html4/struct/text.html#h-9.1
-				if (whitespaceRE.test(content)) {
+				// However, preserve whitespace inside 
 elements where it's semantically significant
+				if (whitespaceRE.test(content) && !isInsidePreElement(current)) {
 					content = ' ';
 				}
 
diff --git a/src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookCells/PositronNotebookMarkdownCell.ts b/src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookCells/PositronNotebookMarkdownCell.ts
index 2a1e58818ff9..69d00f84c04f 100644
--- a/src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookCells/PositronNotebookMarkdownCell.ts
+++ b/src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookCells/PositronNotebookMarkdownCell.ts
@@ -12,7 +12,6 @@ import { PositronNotebookInstance } from '../PositronNotebookInstance.js';
 import { IPositronNotebookMarkdownCell } from './IPositronNotebookCell.js';
 import { ICodeEditor } from '../../../../../editor/browser/editorBrowser.js';
 import { INotebookExecutionStateService } from '../../../notebook/common/notebookExecutionStateService.js';
-import { CellSelectionType } from '../selectionMachine.js';
 
 export class PositronNotebookMarkdownCell extends PositronNotebookCellGeneral implements IPositronNotebookMarkdownCell {
 
@@ -36,18 +35,25 @@ export class PositronNotebookMarkdownCell extends PositronNotebookCellGeneral im
 		);
 	}
 
-	toggleEditor(): void {
+	async toggleEditor(): Promise {
 		const editorStartingOpen = this.editorShown.get();
-		this.editorShown.set(!editorStartingOpen, undefined);
-		// Make sure cell stays selected if we're closing the editor
 		if (editorStartingOpen) {
-			this.select(CellSelectionType.Normal);
+			// Closing the editor - exit editing mode and return to selected state
+			this._instance.selectionStateMachine.exitEditor(this);
+			this.editorShown.set(false, undefined);
+		} else {
+			// Opening the editor - enter editing mode through the selection machine
+			// This will properly handle state transitions and focus management
+			await this._instance.selectionStateMachine.enterEditor(this);
 		}
 	}
 
 	override async showEditor(): Promise {
 		this.editorShown.set(true, undefined);
 		await waitForState(this._editor, (editor) => editor !== undefined);
+		// Wait for the text model to be loaded before returning
+		// This ensures the editor is fully ready for focus operations
+		await this.getTextEditorModel();
 		return super.showEditor();
 	}
 
diff --git a/src/vs/workbench/contrib/positronNotebook/browser/markdownRenderer.ts b/src/vs/workbench/contrib/positronNotebook/browser/markdownRenderer.ts
new file mode 100644
index 000000000000..1ce0110069f1
--- /dev/null
+++ b/src/vs/workbench/contrib/positronNotebook/browser/markdownRenderer.ts
@@ -0,0 +1,96 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (C) 2025 Posit Software, PBC. All rights reserved.
+ *  Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { escape } from '../../../../base/common/strings.js';
+import { ILanguageService } from '../../../../editor/common/languages/language.js';
+import { tokenizeToString } from '../../../../editor/common/languages/textToHtmlTokenizer.js';
+import * as marked from '../../../../base/common/marked/marked.js';
+import { IExtensionService } from '../../../services/extensions/common/extensions.js';
+
+/**
+ * Renders markdown with theme-aware syntax highlighting for Positron notebooks.
+ * Unlike renderMarkdownDocument, this doesn't sanitize aggressively since notebook
+ * content is trusted and needs to support local image paths.
+ */
+export async function renderNotebookMarkdown(
+	content: string,
+	extensionService: IExtensionService,
+	languageService: ILanguageService
+): Promise {
+	const m = new marked.Marked(
+		markedHighlight({
+			async: true,
+			async highlight(code: string, lang: string): Promise {
+				if (!lang) {
+					return escape(code);
+				}
+
+				await extensionService.whenInstalledExtensionsRegistered();
+
+				const languageId = languageService.getLanguageIdByLanguageName(lang)
+					?? languageService.getLanguageIdByLanguageName(lang.split(/\s+|:|,|(?!^)\{|\?]/, 1)[0]);
+
+				return tokenizeToString(languageService, code, languageId);
+			}
+		})
+	);
+
+	return await m.parse(content, { async: true });
+}
+
+/**
+ * Helper for marked.js syntax highlighting integration.
+ *
+ * NOTE: This code is duplicated from markdownDocumentRenderer.ts (MarkedHighlight namespace)
+ * which itself copied it from https://github.com/markedjs/marked-highlight.
+ *
+ * We duplicate it here rather than modifying upstream code to avoid merge conflicts.
+ * The MarkedHighlight namespace is private in upstream, so we can't import it.
+ *
+ * If VSCode upstream exports this utility in the future, we should use that instead.
+ */
+function markedHighlight(options: marked.MarkedOptions & {
+	highlight: (code: string, lang: string) => string | Promise;
+	async?: boolean;
+}): marked.MarkedExtension {
+	if (!options || typeof options.highlight !== 'function') {
+		throw new Error('Must provide highlight function');
+	}
+
+	return {
+		async: !!options.async,
+		walkTokens(token: marked.Token): Promise | void {
+			if (token.type !== 'code') {
+				return;
+			}
+
+			if (options.async) {
+				return Promise.resolve(options.highlight(token.text, token.lang || '')).then(updateToken(token));
+			}
+
+			const code = options.highlight(token.text, token.lang || '');
+			if (code instanceof Promise) {
+				throw new Error('markedHighlight is not set to async but the highlight function is async.');
+			}
+			updateToken(token)(code);
+		},
+		renderer: {
+			code({ text, lang, escaped }: marked.Tokens.Code) {
+				const classAttr = lang ? ` class="language-${escape(lang)}"` : '';
+				text = text.replace(/\n$/, '');
+				return `
${escaped ? text : escape(text)}\n
`; + }, + }, + }; +} + +function updateToken(token: any) { + return (code: string) => { + if (typeof code === 'string' && code !== token.text) { + token.escaped = true; + token.text = code; + } + }; +} diff --git a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CellEditorMonacoWidget.tsx b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CellEditorMonacoWidget.tsx index 684b84498dca..afc192a54292 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CellEditorMonacoWidget.tsx +++ b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CellEditorMonacoWidget.tsx @@ -23,6 +23,7 @@ import { PositronNotebookCellGeneral } from '../PositronNotebookCells/PositronNo import { usePositronReactServicesContext } from '../../../../../base/browser/positronReactRendererContext.js'; import { autorun } from '../../../../../base/common/observable.js'; import { POSITRON_NOTEBOOK_CELL_EDITOR_FOCUSED } from '../ContextKeysManager.js'; +import { SelectionState } from '../selectionMachine.js'; /** * @@ -133,13 +134,22 @@ export function useCellEditorWidget(cell: PositronNotebookCellGeneral) { const disposable = autorun(reader => { cell.editorFocusRequested.read(reader); const editor = cell.editor; + // Check if THIS cell is still the one being edited + // This prevents stale focus requests when user rapidly navigates between cells + const state = instance.selectionStateMachine.state.read(reader); + const shouldFocus = state.type === SelectionState.EditingSelection && state.selected === cell; + + if (!shouldFocus) { + return; + } + if (editor) { editor.focus(); } }); return () => disposable.dispose(); - }, [cell]); + }, [cell, instance.selectionStateMachine]); return { editorPartRef }; } diff --git a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/Markdown.tsx b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/Markdown.tsx index 1984ebec0d6e..e0a8d1c699ec 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/Markdown.tsx +++ b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/Markdown.tsx @@ -16,6 +16,8 @@ import { ExternalLink } from '../../../../../base/browser/ui/ExternalLink/Extern import { localize } from '../../../../../nls.js'; import { createCancelablePromise, raceTimeout } from '../../../../../base/common/async.js'; import { usePositronReactServicesContext } from '../../../../../base/browser/positronReactRendererContext.js'; +import { renderNotebookMarkdown } from '../markdownRenderer.js'; +import { IExtensionService } from '../../../../services/extensions/common/extensions.js'; /** * Component that render markdown content from a string. @@ -56,7 +58,7 @@ function useMarkdown(content: string): MarkdownRenderResults { React.useEffect(() => { const conversionCancellablePromise = createCancelablePromise(() => raceTimeout( - services.commandService.executeCommand('markdown.api.render', content), + renderNotebookMarkdown(content, services.get(IExtensionService), services.languageService), 5000, )); diff --git a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookCellWrapper.tsx b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookCellWrapper.tsx index 404583131ca0..e0823095b903 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookCellWrapper.tsx +++ b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookCellWrapper.tsx @@ -110,10 +110,11 @@ export function NotebookCellWrapper({ cell, actionBarChildren, children }: { return; } + // If already selected, do nothing - maintain selection invariant if (selectionStatus === CellSelectionStatus.Selected) { - cell.deselect(); return; } + const addMode = e.shiftKey || e.ctrlKey || e.metaKey; selectionStateMachine.selectCell(cell, addMode ? CellSelectionType.Add : CellSelectionType.Normal); }} diff --git a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookMarkdownCell.tsx b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookMarkdownCell.tsx index fd1416d68bc5..72bce1a6d287 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookMarkdownCell.tsx +++ b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookMarkdownCell.tsx @@ -15,9 +15,11 @@ import { useObservedValue } from '../useObservedValue.js'; import { Markdown } from './Markdown.js'; import { NotebookCellWrapper } from './NotebookCellWrapper.js'; import { PositronNotebookMarkdownCell } from '../PositronNotebookCells/PositronNotebookMarkdownCell.js'; +import { useNotebookInstance } from '../NotebookInstanceProvider.js'; export function NotebookMarkdownCell({ cell }: { cell: PositronNotebookMarkdownCell }) { + const notebookInstance = useNotebookInstance(); const markdownString = useObservedValue(cell.markdownString); const editorShown = useObservedValue(cell.editorShown); @@ -29,14 +31,21 @@ export function NotebookMarkdownCell({ cell }: { cell: PositronNotebookMarkdownC {editorShown ? : null}
-
{ - cell.toggleEditor(); - }}> +
{ + // Prevent bubbling to wrapper's onClick and default browser behavior + e.stopPropagation(); + e.preventDefault(); + // Enter edit mode for this cell + notebookInstance.selectionStateMachine.enterEditor(cell); + }} + > { markdownString.length > 0 ? :
- Empty markup cell. {editorShown ? '' : 'Double click to edit'} + Empty markup cell. {editorShown ? '' : 'Double click to edit.'}
}
diff --git a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/actionBar/NotebookCellMoreActionsMenu.tsx b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/actionBar/NotebookCellMoreActionsMenu.tsx index d0ba565a43ee..75d595f8f7b2 100644 --- a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/actionBar/NotebookCellMoreActionsMenu.tsx +++ b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/actionBar/NotebookCellMoreActionsMenu.tsx @@ -39,7 +39,11 @@ export function NotebookCellMoreActionsMenu({ const buttonRef = useRef(null); const [isMenuOpen, setIsMenuOpen] = React.useState(false); const commandService = usePositronReactServicesContext().commandService; - const showMoreActionsMenu = () => { + const showMoreActionsMenu = (e: React.MouseEvent) => { + // Prevent click from bubbling to cell wrapper which would deselect the cell + e.preventDefault(); + e.stopPropagation(); + if (!buttonRef.current) { return; }