Refactor Notebook Link Handling (#16473)

* add keep absolute paths instead convert setting

* update tests/config

* refactor links in NotebookLinkHandler

src/sql/workbench/contrib/notebook/browser/cellViews/markdownToolbar.component.ts

@@ -24,6 +24,7 @@ import { URI } from 'vs/base/common/uri';
 import { escape } from 'vs/base/common/strings';
 import { IImageCalloutDialogOptions, ImageCalloutDialog } from 'sql/workbench/contrib/notebook/browser/calloutDialog/imageCalloutDialog';
 import { TextCellEditModes } from 'sql/workbench/services/notebook/common/contracts';
+import { NotebookLinkHandler } from 'sql/workbench/contrib/notebook/browser/notebookLinkHandler';
 
 export const MARKDOWN_TOOLBAR_SELECTOR: string = 'markdown-toolbar-component';
 const linksRegex = /\[(?<text>.+)\]\((?<url>[^ ]+)(?: "(?<title>.+)")?\)/;
@@ -239,19 +240,11 @@ export class MarkdownToolbarComponent extends AngularDisposable {
 			if (this.cellModel.currentMode !== CellEditModes.WYSIWYG) {
 				needsTransform = false;
 			} else {
-				let linkUrl = linkCalloutResult.insertUnescapedLinkUrl;
-				const isAnchorLink = linkUrl.startsWith('#');
-				if (!isAnchorLink) {
-					const isFile = URI.parse(linkUrl).scheme === 'file';
-					if (isFile && !path.isAbsolute(linkUrl)) {
-						const notebookDirName = path.dirname(this.cellModel?.notebookModel?.notebookUri.fsPath);
-						const relativePath = (linkUrl).replace(/\\/g, path.posix.sep);
-						linkUrl = path.resolve(notebookDirName, relativePath);
-					}
-				}
+				let notebookLink = new NotebookLinkHandler(this.cellModel?.notebookModel?.notebookUri, linkCalloutResult.insertUnescapedLinkUrl, this._configurationService);
+				let linkUrl = notebookLink.getLinkUrl();
 				// Otherwise, re-focus on the output element, and insert the link directly.
 				this.output?.nativeElement?.focus();
-				document.execCommand('insertHTML', false, `<a href="${escape(linkUrl)}">${escape(linkCalloutResult?.insertUnescapedLinkLabel)}</a>`);
+				document.execCommand('insertHTML', false, `<a href="${escape(linkUrl)}" is-absolute=${notebookLink.isAbsolutePath}>${escape(linkCalloutResult?.insertUnescapedLinkLabel)}</a>`);
 				return;
 			}
 		} else if (type === MarkdownButtonType.IMAGE_PREVIEW) {
@@ -356,12 +349,8 @@ export class MarkdownToolbarComponent extends AngularDisposable {
 				return '';
 			}
 			const parentNode = anchorNode.parentNode as HTMLAnchorElement;
-			if (parentNode?.protocol === 'file:') {
-				// Pathname starts with / per https://developer.mozilla.org/en-US/docs/Web/API/HTMLAnchorElement/pathname so trim it off
-				return parentNode.pathname?.slice(1) || '';
-			} else {
-				return parentNode.href || '';
-			}
+			const linkHandler = new NotebookLinkHandler(this.cellModel?.notebookModel?.notebookUri, parentNode, this._configurationService);
+			return linkHandler.getLinkUrl();
 		} else {
 			const editorControl = this.getCellEditorControl();
 			const selection = editorControl?.getSelection();

src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts

@@ -177,7 +177,7 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
 		this.updateTheme(this.themeService.getColorTheme());
 		this.setFocusAndScroll();
 		this.cellModel.isEditMode = false;
-		this._htmlMarkdownConverter = new HTMLMarkdownConverter(this.notebookUri);
+		this._htmlMarkdownConverter = this._instantiationService.createInstance(HTMLMarkdownConverter, this.notebookUri);
 		this._register(this.cellModel.onOutputsChanged(e => {
 			this.updatePreview();
 		}));

src/sql/workbench/contrib/notebook/browser/htmlMarkdownConverter.ts

@@ -7,7 +7,8 @@ import TurndownService = require('turndown');
 import { URI } from 'vs/base/common/uri';
 import * as path from 'vs/base/common/path';
 import * as turndownPluginGfm from 'sql/workbench/contrib/notebook/browser/turndownPluginGfm';
-import { replaceInvalidLinkPath } from 'sql/workbench/contrib/notebook/common/utils';
+import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
+import { findPathRelativeToContent, NotebookLinkHandler } from 'sql/workbench/contrib/notebook/browser/notebookLinkHandler';
 
 // These replacements apply only to text. Here's how it's handled from Turndown:
 // if (node.nodeType === 3) {
@@ -30,10 +31,11 @@ const markdownReplacements = [
 	[/</g, '\\<'], // Added to ensure sample text like <hello> is escaped
 	[/>/g, '\\>'], // Added to ensure sample text like <hello> is escaped
 ];
+
 export class HTMLMarkdownConverter {
 	private turndownService: TurndownService;
 
-	constructor(private notebookUri: URI) {
+	constructor(private notebookUri: URI, @IConfigurationService private configurationService: IConfigurationService,) {
 		this.turndownService = new TurndownService({ 'emDelimiter': '_', 'bulletListMarker': '-', 'headingStyle': 'atx', blankReplacement: blankReplacement });
 		this.setTurndownOptions();
 	}
@@ -132,27 +134,8 @@ export class HTMLMarkdownConverter {
 		this.turndownService.addRule('a', {
 			filter: 'a',
 			replacement: (content, node) => {
-				let href = node.href;
-				let notebookLink: URI | undefined;
-				const isAnchorLinkInFile = (node.attributes.href?.nodeValue.startsWith('#') || href.includes('#')) && href.startsWith('file://');
-				if (isAnchorLinkInFile) {
-					notebookLink = getUriAnchorLink(node, this.notebookUri);
-				} else {
-					//On Windows, if notebook is not trusted then the href attr is removed for all non-web URL links
-					// href contains either a hyperlink or a URI-encoded absolute path. (See resolveUrls method in notebookMarkdown.ts)
-					notebookLink = href ? URI.parse(href) : URI.file(node.title);
-				}
-				const notebookFolder = this.notebookUri ? path.join(path.dirname(this.notebookUri.fsPath), path.sep) : '';
-				if (notebookLink.fsPath !== this.notebookUri.fsPath) {
-					let relativePath = findPathRelativeToContent(notebookFolder, notebookLink);
-					if (relativePath) {
-						return `[${node.innerText}](${relativePath})`;
-					}
-				} else if (notebookLink?.fragment) {
-					// if the anchor link is to a section in the same notebook then just add the fragment
-					return `[${content}](${notebookLink.fragment})`;
-				}
-
+				const linkHandler = new NotebookLinkHandler(this.notebookUri, node, this.configurationService);
+				const href = linkHandler.getLinkUrl();
 				return `[${content}](${href})`;
 			}
 		});
@@ -304,25 +287,6 @@ function isInsideTable(node): boolean {
 	return node.parentNode?.nodeName === 'TH' || node.parentNode?.nodeName === 'TD';
 }
 
-export function findPathRelativeToContent(notebookFolder: string, contentPath: URI | undefined): string {
-	if (notebookFolder) {
-		if (contentPath?.scheme === 'file') {
-			let relativePath = contentPath.fragment ? path.relative(notebookFolder, contentPath.fsPath).concat('#', contentPath.fragment) : path.relative(notebookFolder, contentPath.fsPath);
-			//if path contains whitespaces then it's not identified as a link
-			relativePath = relativePath.replace(/\s/g, '%20');
-			// if relativePath contains improper directory format due to marked js parsing returning an invalid path (ex. ....\) then we need to replace it to ensure the directories are formatted properly (ex. ..\..\)
-			relativePath = replaceInvalidLinkPath(relativePath);
-			if (relativePath.startsWith(path.join('..', path.sep)) || relativePath.startsWith(path.join('.', path.sep))) {
-				return relativePath;
-			} else {
-				// if the relative path does not contain ./ at the beginning, we need to add it so it's recognized as a link
-				return `.${path.join(path.sep, relativePath)}`;
-			}
-		}
-	}
-	return '';
-}
-
 export function addHighlightIfYellowBgExists(node, content: string): string {
 	if (node?.style?.backgroundColor === 'yellow') {
 		return '<mark>' + content + '</mark>';
@@ -330,14 +294,3 @@ export function addHighlightIfYellowBgExists(node, content: string): string {
 	return content;
 }
 
-export function getUriAnchorLink(node, notebookUri: URI): URI {
-	const sectionLinkToAnotherFile = node.href.includes('#') && !node.attributes.href?.nodeValue.startsWith('#');
-	if (sectionLinkToAnotherFile) {
-		let absolutePath = !path.isAbsolute(node.attributes.href?.nodeValue) ? path.resolve(path.dirname(notebookUri.fsPath), node.attributes.href?.nodeValue) : node.attributes.href?.nodeValue;
-		// if section link is different from the current notebook
-		return URI.file(absolutePath);
-	} else {
-		// else build an uri using the current notebookUri
-		return URI.from({ scheme: 'file', path: notebookUri.path, fragment: node.attributes.href?.nodeValue });
-	}
-}

src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts

@@ -335,6 +335,11 @@ configurationRegistry.registerConfiguration({
 			'default': 200,
 			'minimum': 10,
 			'description': localize('notebook.maxRichTextUndoHistory', "The maximum number of changes stored in the undo history for the notebook Rich Text editor.")
+		},
+		'notebook.useAbsoluteFilePaths': {
+			'type': 'boolean',
+			'default': false,
+			'description': localize('notebook.useAbsoluteFilePaths', "Use absolute file paths when linking to other notebooks.")
 		}
 	}
 });

src/sql/workbench/contrib/notebook/browser/notebookLinkHandler.ts

@@ -0,0 +1,145 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { URI } from 'vs/base/common/uri';
+import * as path from 'vs/base/common/path';
+import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
+import { replaceInvalidLinkPath } from 'sql/workbench/contrib/notebook/common/utils';
+import { isWindows } from 'vs/base/common/platform';
+
+const keepAbsolutePathConfigName = 'notebook.keepAbsolutePath';
+
+export class NotebookLinkHandler {
+	private _notebookUriLink: URI;
+	private _href: string;
+	private _notebookDirectory: string;
+	private _isAnchorLink: boolean;
+	private _isFile: boolean;
+	public readonly isAbsolutePath: boolean;
+
+	constructor(
+		private _notebookURI: URI,
+		private _link: string | HTMLAnchorElement,
+		@IConfigurationService private _configurationService: IConfigurationService,
+	) {
+		if (typeof this._link === 'string') {
+			this._notebookUriLink = URI.parse(this._link);
+			this._isFile = this._notebookUriLink.scheme === 'file';
+			this.isAbsolutePath = path.isAbsolute(this._link);
+			this._isAnchorLink = this._link.includes('#') && this._isFile;
+		} else {
+			// HTMLAnchorElement
+			// windows files need to use the link.href instead as it contains the file:// prefix
+			// which enables us to get the proper relative path
+			if (isWindows) {
+				this._href = this._link.href;
+			} else {
+				this._href = this._link.attributes['href']?.nodeValue;
+			}
+			this._notebookUriLink = this._href ? URI.parse(this._href) : undefined;
+			this._isFile = this._link.protocol === 'file:';
+			this._isAnchorLink = this._notebookUriLink?.fragment ? true : false;
+			this.isAbsolutePath = this._link.attributes['is-absolute']?.nodeValue === 'true' ? true : false;
+		}
+		this._notebookDirectory = this._notebookURI ? path.dirname(this._notebookURI.fsPath) : '';
+	}
+
+	/**
+	 * Function to get the link for LinkCalloutDialog or htmlMarkdownConverter
+	 * When a user inserts a new link via the LinkCalloutDialog it will go through the string case to
+	 * get the absolute path of the file and then will be converted to anchor element that will be called again
+	 * to the object case in which we will find the relative path of the file unless the user has the
+	 * keep absolute setting enabled then we don't convert absolute paths to relative paths
+	 * @returns the file link or web link
+	 */
+	public getLinkUrl(): string {
+		// cases where we only have the href link
+		if (typeof this._link === 'string') {
+			// Does not convert absolute path to relative path
+			if (this._isFile && this.isAbsolutePath && this._configurationService.getValue(keepAbsolutePathConfigName) === true) {
+				return this._link;
+			}
+			// sets the string to absolute path to be used to resolve
+			if (this._isFile && !this.isAbsolutePath && !this._isAnchorLink) {
+				const relativePath = (this._link).replace(/\\/g, path.posix.sep);
+				const linkUrl = path.resolve(this._notebookDirectory, relativePath);
+				return linkUrl;
+			}
+			/**
+			 * We return the absolute path for the link so that it will get used in the as the href for the anchor HTML element
+			 * (in linkCalloutDialog document.execCommand('insertHTML') and therefore will call getLinkURL() with HTMLAnchorElement to then get the relative path
+			*/
+			return this._link;
+		} else {
+			// cases where we pass the HTMLAnchorElement
+			if (this._notebookUriLink && this._isFile) {
+				let targetUri: URI;
+				// Does not convert absolute path to relative path if keep Absolute Path setting is enabled
+				if (this.isAbsolutePath && this._configurationService.getValue(keepAbsolutePathConfigName) === true) {
+					return escape(this._href);
+				} else {
+					if (this._isAnchorLink) {
+						targetUri = this.getUriAnchorLink(this._link, this._notebookURI);
+					} else {
+						//On Windows, if notebook is not trusted then the href attr is removed for all non-web URL links
+						// href contains either a hyperlink or a URI-encoded absolute path. (See resolveUrls method in notebookMarkdown.ts)
+						targetUri = this._link ? this._notebookUriLink : URI.file(this._link.title);
+					}
+					// returns relative path of target notebook to the current notebook directory
+					if (this._notebookUriLink.fsPath !== this._notebookURI.fsPath && !targetUri?.fragment) {
+						return findPathRelativeToContent(this._notebookDirectory, targetUri);
+					} else {
+						// if the anchor link is to a section in the same notebook then just add the fragment
+						return targetUri.fragment;
+					}
+				}
+			}
+			// Web links
+			return this._href || '';
+		}
+	}
+
+	/**
+	 * Creates a URI for for a link with a anchor (#)
+	 * @param node is the HTMLAnchorElement of the target notebook
+	 * @param notebookUri is current notebook URI
+	 * @returns URI of the link with the anchor
+	 */
+	public getUriAnchorLink(node, notebookUri: URI): URI {
+		const sectionLinkToAnotherFile = node.href.includes('#') && !node.attributes.href?.nodeValue.startsWith('#');
+		if (sectionLinkToAnotherFile) {
+			let absolutePath = !path.isAbsolute(node.attributes.href?.nodeValue) ? path.resolve(path.dirname(notebookUri.fsPath), node.attributes.href?.nodeValue) : node.attributes.href?.nodeValue;
+			// if section link is different from the current notebook
+			return URI.file(absolutePath);
+		} else {
+			// else build an uri using the current notebookUri
+			return URI.from({ scheme: 'file', path: notebookUri.path, fragment: node.attributes.href?.nodeValue });
+		}
+	}
+
+}
+
+/**
+ * Finds the Relative Path from current notebook folder to target (linked) notebook
+ * @param notebookFolder is the current notebook directory
+ * @param contentPath is the URI path to the notebook we are linking to
+ * @returns relative path from the current notebook to the target notebook
+ */
+export function findPathRelativeToContent(notebookFolder: string, contentPath: URI | undefined): string {
+	if (contentPath?.scheme === 'file') {
+		let relativePath = contentPath.fragment ? path.relative(notebookFolder, contentPath.fsPath).concat('#', contentPath.fragment) : path.relative(notebookFolder, contentPath.fsPath);
+		//if path contains whitespaces then it's not identified as a link
+		relativePath = relativePath.replace(/\s/g, '%20');
+		// if relativePath contains improper directory format due to marked js parsing returning an invalid path (ex. ....\) then we need to replace it to ensure the directories are formatted properly (ex. ..\..\)
+		relativePath = replaceInvalidLinkPath(relativePath);
+		if (relativePath.startsWith(path.join('..', path.sep)) || relativePath.startsWith(path.join('.', path.sep))) {
+			return relativePath;
+		} else {
+			// if the relative path does not contain ./ at the beginning, we need to add it so it's recognized as a link
+			return `.${path.join(path.sep, relativePath)}`;
+		}
+	}
+	return '';
+}

src/sql/workbench/contrib/notebook/browser/outputs/notebookMarkdown.ts

@@ -108,6 +108,8 @@ export class NotebookMarkdownRenderer {
 			return '<img ' + attributes.join(' ') + '>';
 		};
 		renderer.link = (href: string, title: string, text: string): string => {
+			// check for isAbsolute prior to escaping and replacement
+			let hrefAbsolute: boolean = path.isAbsolute(href);
 			href = this.cleanUrl(!markdown.isTrusted, notebookFolder, href);
 			if (href === null) {
 				return text;
@@ -120,7 +122,7 @@ export class NotebookMarkdownRenderer {
 			// only remove markdown escapes if it's a hyperlink, filepath usually can start with .{}_
 			// and the below function escapes them if it encounters in the path.
 			// dev note: using path.isAbsolute instead of isPathLocal since the latter accepts resolver (IRenderMime.IResolver) to check isLocal
-			if (!path.isAbsolute(href)) {
+			if (!hrefAbsolute) {
 				href = removeMarkdownEscapes(href);
 			}
 			if (
@@ -143,7 +145,7 @@ export class NotebookMarkdownRenderer {
 					.replace(/>/g, '&gt;')
 					.replace(/"/g, '&quot;')
 					.replace(/'/g, '&#39;');
-				return `<a href=${href} data-href="${href}" title="${title || href}">${text}</a>`;
+				return `<a href=${href} data-href="${href}" title="${title || href}" is-absolute=${hrefAbsolute}>${text}</a>`;
 			}
 		};
 		renderer.paragraph = (text): string => {