Fix #3481 Notebook: Markdown coloring appears incorrect (#3499)

- Set markdown as language for markdown cell
- Fix issue where after loading language from cell metadata, always override it. This made the "language" feature irrelevant in the cell.
- Fixed tests with new behavior (assumption: cell-level language overrides notebook-level definition) and added new test to cover this too

src/sql/parts/notebook/models/cell.ts

@@ -38,15 +38,15 @@ export class CellModel implements ICellModel {
 	constructor(private factory: IModelFactory, cellData?: nb.ICellContents, private _options?: ICellModelOptions) {
 		this.id = `${modelId++}`;
 		CellModel.CreateLanguageMappings();
-		// Do nothing for now
 		if (cellData) {
+			// Read in contents if available
 			this.fromJSON(cellData);
 		} else {
 			this._cellType = CellTypes.Code;
 			this._source = '';
 		}
 		this._isEditMode = this._cellType !== CellTypes.Markdown;
-		this.setDefaultLanguage();
+		this.ensureDefaultLanguage();
 		if (_options && _options.isTrusted) {
 			this._isTrusted = true;
 		} else {
@@ -284,7 +284,7 @@ export class CellModel implements ICellModel {
 		}
 		this._cellType = cell.cell_type;
 		this._source = Array.isArray(cell.source) ? cell.source.join('') : cell.source;
-		this._language = (cell.metadata && cell.metadata.language) ? cell.metadata.language : 'python';
+		this.setLanguageFromContents(cell);
 		if (cell.outputs) {
 			for (let output of cell.outputs) {
 				// For now, we're assuming it's OK to save these as-is with no modification
@@ -293,6 +293,15 @@ export class CellModel implements ICellModel {
 		}
 	}
 
+	private setLanguageFromContents(cell: nb.ICellContents): void {
+		if (cell.cell_type === CellTypes.Markdown) {
+			this._language = 'markdown';
+		} else if (cell.metadata && cell.metadata.language) {
+			this._language = cell.metadata.language;
+		}
+		// else skip, we set default language anyhow
+	}
+
 	private addOutput(output: nb.ICellOutput) {
 		this._normalize(output);
 		this._outputs.push(output);
@@ -327,8 +336,32 @@ export class CellModel implements ICellModel {
 		return undefined;
 	}
 
-	private setDefaultLanguage(): void {
+	/**
-		this._language = 'python';
+	 * Ensures there is a default language set, if none was already defined.
+	 * Will read information from the overall Notebook (passed as options to the model), or
+	 * if all else fails default back to python.
+	 *
+	 */
+	private ensureDefaultLanguage(): void {
+		// See if language is already set / is known based on cell type
+		if (this.hasLanguage()) {
+			return;
+		}
+		if (this._cellType === CellTypes.Markdown) {
+			this._language = 'markdown';
+			return;
+		}
+
+		// try set it based on overall Notebook language
+		this.trySetLanguageFromLangInfo();
+
+		// fallback to python
+		if (!this._language) {
+			this._language = 'python';
+		}
+	}
+
+	private trySetLanguageFromLangInfo() {
 		// In languageInfo, set the language to the "name" property
 		// If the "name" property isn't defined, check the "mimeType" property
 		// Otherwise, default to python as the language
@@ -338,16 +371,25 @@ export class CellModel implements ICellModel {
 				// check the LanguageMapping to determine if a mapping is necessary (example 'pyspark' -> 'python')
 				if (CellModel.LanguageMapping[languageInfo.name]) {
 					this._language = CellModel.LanguageMapping[languageInfo.name];
-				} else {
+				}
+				else {
 					this._language = languageInfo.name;
 				}
-			} else if (languageInfo.mimetype) {
+			}
+			else if (languageInfo.mimetype) {
 				this._language = languageInfo.mimetype;
 			}
 		}
-		let mimeTypePrefix = 'x-';
+
-		if (this._language.includes(mimeTypePrefix)) {
+		if (this._language) {
-			this._language = this._language.replace(mimeTypePrefix, '');
+			let mimeTypePrefix = 'x-';
+			if (this._language.includes(mimeTypePrefix)) {
+				this._language = this._language.replace(mimeTypePrefix, '');
+			}
 		}
 	}
+
+	private hasLanguage(): boolean {
+		return !!this._language;
+	}
 }

src/sqltest/parts/notebook/model/cell.test.ts

@@ -93,7 +93,7 @@ describe('Cell Model', function (): void {
         let cellData: nb.ICellContents = {
             cell_type: CellTypes.Code,
             source: 'print(\'1\')',
-            metadata: { language: 'python'},
+            metadata: {  },
             execution_count: 1
         };
 
@@ -105,6 +105,22 @@ describe('Cell Model', function (): void {
         let cell = factory.createCell(cellData, { notebook: notebookModel, isTrusted: false });
         should(cell.language).equal('scala');
     });
+    it('Should keep cell language as python if cell has language override', async function (): Promise<void> {
+        let cellData: nb.ICellContents = {
+            cell_type: CellTypes.Code,
+            source: 'print(\'1\')',
+            metadata: { language: 'python'},
+            execution_count: 1
+        };
+
+        let notebookModel = new NotebookModelStub({
+            name: 'scala',
+            version: '',
+            mimetype: ''
+        });
+        let cell = factory.createCell(cellData, { notebook: notebookModel, isTrusted: false });
+        should(cell.language).equal('python');
+    });
 
     it('Should set cell language to python if no language defined', async function (): Promise<void> {
         let cellData: nb.ICellContents = {
@@ -127,7 +143,7 @@ describe('Cell Model', function (): void {
         let cellData: nb.ICellContents = {
             cell_type: CellTypes.Code,
             source: 'std::cout << "hello world";',
-            metadata: { language: 'python'},
+            metadata: { },
             execution_count: 1
         };
 
@@ -144,7 +160,7 @@ describe('Cell Model', function (): void {
         let cellData: nb.ICellContents = {
             cell_type: CellTypes.Code,
             source: 'print(\'1\')',
-            metadata: { language: 'python'},
+            metadata: { },
             execution_count: 1
         };