From fa71407f3656b0bf65d40e94a20513d56bcf3c61 Mon Sep 17 00:00:00 2001
From: Profpatsch <mail@profpatsch.de>
Date: Mon, 26 Mar 2018 17:31:05 +0200
Subject: [PATCH] lib/generators: introduce a sane default for `mkValueString`

So far, `mkValueString` defaulted to `toString`,
which is a bad match for most configuration file formats,
especially because how booleans are formatted.
This also improves error messages for unsupported types.

Add a test to codify the formatting.
---
 lib/generators.nix | 26 +++++++++++++++++++++++++-
 lib/tests/misc.nix | 26 ++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/lib/generators.nix b/lib/generators.nix
index 6adf0c2afbc7..d1a8f6bf8dcd 100644
--- a/lib/generators.nix
+++ b/lib/generators.nix
@@ -28,6 +28,30 @@ rec {
 
   ## -- HELPER FUNCTIONS & DEFAULTS --
 
+  /* Convert a value to a sensible default string representation.
+   * The builtin `toString` function has some strange defaults,
+   * suitable for bash scripts but not much else.
+   */
+  mkValueStringDefault = {}: v: with builtins;
+    let err = t: v: abort
+          ("generators.mkValueStringDefault: " +
+           "${t} not supported: ${toPretty {} v}");
+    in   if isInt      v then toString v
+    # we default to not quoting strings
+    else if isString   v then v
+    # isString returns "1", which is not a good default
+    else if true  ==   v then "true"
+    # here it returns to "", which is even less of a good default
+    else if false ==   v then "false"
+    else if null  ==   v then "null"
+    # if you have lists you probably want to replace this
+    else if isList     v then err "lists" v
+    # same as for lists, might want to replace
+    else if isAttrs    v then err "attrsets" v
+    else if isFunction v then err "functions" v
+    else err "this value is" (toString v);
+
+
   /* Generate a line of key k and value v, separated by
    * character sep. If sep appears in k, it is escaped.
    * Helper for synaxes with different separators.
@@ -38,7 +62,7 @@ rec {
    * > "f\:oo:bar"
    */
   mkKeyValueDefault = {
-    mkValueString ? toString
+    mkValueString ? mkValueStringDefault {}
   }: sep: k: v:
     "${libStr.escape [sep] k}${sep}${mkValueString v}";
 
diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix
index e10aea48e48e..5f19dd63f2d8 100644
--- a/lib/tests/misc.nix
+++ b/lib/tests/misc.nix
@@ -207,6 +207,29 @@ runTests {
     expected = ''f\:oo:bar'';
   };
 
+  testMkValueString = {
+    expr = let
+      vals = {
+        int = 42;
+        string = ''fo"o'';
+        bool = true;
+        bool2 = false;
+        null = null;
+        # float = 42.23; # floats are strange
+      };
+      in mapAttrs
+        (const (generators.mkValueStringDefault {}))
+        vals;
+    expected = {
+      int = "42";
+      string = ''fo"o'';
+      bool = "true";
+      bool2 = "false";
+      null = "null";
+      # float = "42.23" true false [ "bar" ] ]'';
+    };
+  };
+
   testToKeyValue = {
     expr = generators.toKeyValue {} {
       key = "value";
@@ -249,6 +272,8 @@ runTests {
       "section 1" = {
         attribute1 = 5;
         x = "Me-se JarJar Binx";
+        # booleans are converted verbatim by default
+        boolean = false;
       };
       "foo[]" = {
         "he\\h=he" = "this is okay";
@@ -260,6 +285,7 @@ runTests {
 
       [section 1]
       attribute1=5
+      boolean=false
       x=Me-se JarJar Binx
     '';
   };