From 1a3006965297654365f1ac18ebfe6aeae2e9b4cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= <radoslaw.wasko@enso.org>
Date: Wed, 22 Jun 2022 16:37:46 +0200
Subject: [PATCH] Fix quoting in always mode

---
 .../org/enso/table/write/DelimitedWriter.java | 55 ++++++++++++++-----
 .../Table_Tests/src/Delimited_Write_Spec.enso |  8 +--
 2 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/std-bits/table/src/main/java/org/enso/table/write/DelimitedWriter.java b/std-bits/table/src/main/java/org/enso/table/write/DelimitedWriter.java
index 63f6ae28d5cd4..a69ab20062b06 100644
--- a/std-bits/table/src/main/java/org/enso/table/write/DelimitedWriter.java
+++ b/std-bits/table/src/main/java/org/enso/table/write/DelimitedWriter.java
@@ -2,11 +2,13 @@
 
 import org.enso.table.data.table.Table;
 import org.enso.table.formatting.DataFormatter;
-import org.enso.table.problems.WithProblems;
 
 import java.io.IOException;
 import java.io.Writer;
-import java.util.List;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZonedDateTime;
 
 public class DelimitedWriter {
   private static final char NEWLINE = '\n';
@@ -98,9 +100,10 @@ public void write(Table table) throws IOException {
     assert numberOfColumns == columnFormatters.length;
 
     if (writeHeaders) {
+      boolean quoteAllHeaders = writeQuoteBehavior == WriteQuoteBehavior.ALWAYS;
       for (int col = 0; col < numberOfColumns; ++col) {
         boolean isLast = col == numberOfColumns - 1;
-        writeCell(table.getColumns()[col].getName(), isLast);
+        writeCell(table.getColumns()[col].getName(), isLast, quoteAllHeaders);
       }
     }
 
@@ -110,19 +113,36 @@ public void write(Table table) throws IOException {
         boolean isLast = col == numberOfColumns - 1;
         Object cellValue = table.getColumns()[col].getStorage().getItemBoxed(row);
         String formatted = columnFormatters[col].format(cellValue);
-        writeCell(formatted, isLast);
+        boolean wantsQuoting =
+            writeQuoteBehavior == WriteQuoteBehavior.ALWAYS && wantsQuotesInAlwaysMode(cellValue);
+        writeCell(formatted, isLast, wantsQuoting);
       }
     }
 
     output.flush();
   }
 
+  private boolean wantsQuotesInAlwaysMode(Object value) {
+    return value instanceof String || !isNonTextPrimitive(value);
+  }
+
+  private boolean isNonTextPrimitive(Object value) {
+    return value instanceof Long
+        || value instanceof Double
+        || value instanceof Boolean
+        || value instanceof LocalDate
+        || value instanceof LocalDateTime
+        || value instanceof LocalTime
+        || value instanceof ZonedDateTime;
+  }
+
   private boolean quotingEnabled() {
     return writeQuoteBehavior != WriteQuoteBehavior.NEVER;
   }
 
-  private void writeCell(String value, boolean isLastInRow) throws IOException {
-    String processed = value == null ? "" : quotingEnabled() ? quote(value) : value;
+  private void writeCell(String value, boolean isLastInRow, boolean wantsQuoting)
+      throws IOException {
+    String processed = value == null ? "" : quotingEnabled() ? quote(value, wantsQuoting) : value;
     output.write(processed);
     if (isLastInRow) {
       output.write(NEWLINE);
@@ -131,18 +151,27 @@ private void writeCell(String value, boolean isLastInRow) throws IOException {
     }
   }
 
-  private String quote(String value) {
-    if (value.isEmpty()) {
-      return emptyValue;
-    }
-
+  private boolean needsQuoting(String value) {
     boolean containsDelimiter = value.indexOf(delimiter) >= 0;
     boolean containsQuote = value.indexOf(quoteChar) >= 0;
     boolean containsQuoteEscape = value.indexOf(quoteEscapeChar) >= 0;
 
-    boolean needsQuoting = containsDelimiter || containsQuote || containsQuoteEscape;
+    return containsDelimiter || containsQuote || containsQuoteEscape;
+  }
+
+  /**
+   * Wraps the value in quotes, escaping any characters if necessary.
+   *
+   * <p>The {@code wantsQuoting} parameter allows to request quoting even if it wouldn't normally be
+   * necessary. This is used to implement the `always_quote` mode for text and custom objects.
+   */
+  private String quote(String value, boolean wantsQuoting) {
+    if (value.isEmpty()) {
+      return emptyValue;
+    }
 
-    if (!needsQuoting) {
+    boolean shouldQuote = wantsQuoting || needsQuoting(value);
+    if (!shouldQuote) {
       return value;
     }
 
diff --git a/test/Table_Tests/src/Delimited_Write_Spec.enso b/test/Table_Tests/src/Delimited_Write_Spec.enso
index 2c9d57ecc2ae1..a8776293014a3 100644
--- a/test/Table_Tests/src/Delimited_Write_Spec.enso
+++ b/test/Table_Tests/src/Delimited_Write_Spec.enso
@@ -121,11 +121,11 @@ spec =
             file.delete_if_exists
             table.write file format
             expected_text = """
-                "The Column ""Name""","B","C","D","E"
+                "The Column \"Name\"","B","C","D","E"
                 "foo",1.0,"foo",1,
-                "'bar'","1""000""000.5","[[[My Type :: 44]]]",2,13:55:00
-                """baz""",2.2,"Tue, 21 Jun 2022",3,
-                "one, two, three",-1.5,42,"4""000",
+                "'bar'","1\"000\"000.5","[[[My Type :: 44]]]",2,13:55:00
+                "\"baz\"",2.2,"Tue, 21 Jun 2022",3,
+                "one, two, three",-1.5,42,"4\"000",
             text = File.read_text file
             text.should_equal expected_text+'\n'