Clean up notebook export code (#1565)

* Add null checks when converting notebooks to SQL, and SQL to notebooks.

* Remove unnecessary newlines and spaces from multi-line comments.

* Use the current environment's newline characters when converting cell text to a SQL query.

* Add additional unit tests for notebook conversion.

src/Microsoft.SqlTools.ServiceLayer/NotebookConvert/NotebookConvertService.cs

@@ -15,6 +15,7 @@ using Microsoft.SqlTools.ServiceLayer.Workspace;
 using Newtonsoft.Json;
 using Microsoft.SqlServer.TransactSql.ScriptDom;
 using System.IO;
+using System.Runtime.InteropServices;
 
 namespace Microsoft.SqlTools.ServiceLayer.NotebookConvert
 {
@@ -115,10 +116,10 @@ namespace Microsoft.SqlTools.ServiceLayer.NotebookConvert
 
         #endregion // Convert Handlers
 
-        internal static NotebookDocument ConvertSqlToNotebook(string sql)
+        internal static NotebookDocument ConvertSqlToNotebook(string? sql)
         {
             // Notebooks use \n so convert any other newlines now
-            sql = sql.Replace("\r\n", "\n");
+            sql = sql?.Replace("\r\n", "\n") ?? string.Empty;
 
             var doc = new NotebookDocument
             {
@@ -200,8 +201,13 @@ namespace Microsoft.SqlTools.ServiceLayer.NotebookConvert
                         commentBlock = commentBlock.Remove(commentBlock.Length - 2);
                     }
 
-                    doc.Cells.Add(GenerateMarkdownCell(commentBlock.Trim()));
+                    // Trim off extra spaces for each line. This helps keep comment asterisks aligned on the 
-                } else if (fragment.TokenType == NotebookTokenType.SinglelineComment)
+                    // same column for multiline comments.
+                    var commentLines = commentBlock.Trim().Split("\n").Select(comment => comment.Trim());
+                    commentBlock = string.Join("\n", commentLines);
+                    doc.Cells.Add(GenerateMarkdownCell(commentBlock));
+                }
+                else if (fragment.TokenType == NotebookTokenType.SinglelineComment)
                 {
                     string commentBlock = fragment.Text;
                     // Trim off the starting comment token (--)
@@ -247,21 +253,39 @@ namespace Microsoft.SqlTools.ServiceLayer.NotebookConvert
         /// Converts a Notebook document into a single string that can be inserted into a SQL
         /// query.
         /// </summary>
-        private static string ConvertNotebookDocToSql(NotebookDocument doc)
+        internal static string ConvertNotebookDocToSql(NotebookDocument? doc)
         {
-            // Add an extra blank line between each block for readability
+            if (doc?.Cells == null)
-            return string.Join(Environment.NewLine + Environment.NewLine, doc.Cells.Select(cell =>
             {
-                return cell.CellType switch
+                return string.Empty;
+            }
+            else
+            {
+                // Add an extra blank line between each block for readability
+                return string.Join(Environment.NewLine + Environment.NewLine, doc.Cells.Select(cell =>
                 {
-                    // Markdown is text so wrapped in a comment block
+                    // Notebooks use \n newlines, so convert the cell source to \r\n if running on Windows.
-                    "markdown" => $@"/*
+                    IEnumerable<string> cellSource;
-{string.Join(Environment.NewLine, cell.Source)}
+                    if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+                    {
+                        cellSource = cell.Source.Select(text => text.Replace("\n", Environment.NewLine));
+                    }
+                    else
+                    {
+                        cellSource = cell.Source;
+                    }
+
+                    return cell.CellType switch
+                    {
+                        // Markdown is text so wrapped in a comment block
+                        "markdown" => $@"/*
+{string.Join("", cellSource)}
 */",
-                    // Everything else (just code blocks for now) is left as is
+                        // Everything else (just code blocks for now) is left as is
-                    _ => string.Join("", cell.Source),
+                        _ => string.Join("", cellSource),
-                };
+                    };
-            }));
+                }));
+            }
         }
     }
 

test/Microsoft.SqlTools.ServiceLayer.UnitTests/NotebookConvert/NotebookConvertServiceTests.cs

@@ -13,10 +13,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.NotebookConvert
     [TestFixture]
     public class NotebookConvertServiceTests
     {
-        [Test]
+        private const string sampleSqlQuery = @"
-        public void ConvertSqlToNotebook()
-        {
-            var sql = @"
 /* 
  * Initial multiline comment
  */ 
@@ -39,7 +36,7 @@ FROM sys.databases
  */
 ";
 
-            var expectedNotebook = @"{
+        private const string sampleNotebook = @"{
   ""metadata"": {
     ""kernelspec"": {
       ""name"": ""SQL"",
@@ -90,16 +87,82 @@ FROM sys.databases
       ""cell_type"": ""markdown"",
       ""source"": [
         ""* Ending multiline  \n"",
-        "" * comment""
+        ""* comment""
       ]
     }
   ]
 }";
-
+        [Test]
-            var notebook = NotebookConvertService.ConvertSqlToNotebook(sql);
+        public void ConvertSqlToNotebook()
+        {
+            var notebook = NotebookConvertService.ConvertSqlToNotebook(sampleSqlQuery);
             var notebookString = JsonConvert.SerializeObject(notebook, Formatting.Indented);
-            Assert.AreEqual(expectedNotebook, notebookString);
+            Assert.That(notebookString, Is.EqualTo(sampleNotebook));
         }
 
+        [Test]
+        public void ConvertNullSqlToNotebook()
+        {
+            var emptyNotebook = @"{
+  ""metadata"": {
+    ""kernelspec"": {
+      ""name"": ""SQL"",
+      ""display_name"": ""SQL"",
+      ""language"": ""sql""
+    },
+    ""language_info"": {
+      ""name"": ""sql"",
+      ""version"": """"
+    }
+  },
+  ""nbformat_minor"": 2,
+  ""nbformat"": 4,
+  ""cells"": []
+}";
+            var notebook = NotebookConvertService.ConvertSqlToNotebook(null);
+            var notebookString = JsonConvert.SerializeObject(notebook, Formatting.Indented);
+            Assert.That(notebookString, Is.EqualTo(emptyNotebook));
+        }
+
+        [Test]
+        public void ConvertNotebookToSql()
+        {
+            var expectedSqlQuery = @"/*
+* Initial multiline comment
+*/
+
+/*
+Comment before batch
+*/
+
+SELECT * FROM sys.databases
+
+-- Compare Row Counts in Tables From Two Different Databases With the Same Schema
+
+SELECT * -- inline single line comment
+/* inline multiline
+ * comment
+ */
+FROM sys.databases
+
+/*
+ending single line comment
+*/
+
+/*
+* Ending multiline  
+* comment
+*/";
+            var notebook = JsonConvert.DeserializeObject<NotebookDocument>(sampleNotebook);
+            var query = NotebookConvertService.ConvertNotebookDocToSql(notebook);
+            Assert.That(query, Is.EqualTo(expectedSqlQuery));
+        }
+
+        [Test]
+        public void ConvertNullNotebookToSql()
+        {
+            var query = NotebookConvertService.ConvertNotebookDocToSql(null);
+            Assert.That(query, Is.EqualTo(string.Empty));
+        }
     }
 }