Don't confuse empty configuration variables with booleans

Config was confusing the following two variables when writing the
file back to text format:

  [my]
    empty =
    enabled

When parsed, we say that my.empty has 1 value, null, and my.enabled
is an empty string value that in boolean context should be evaluated
as true.

Saving this configuration file back to text format was ignoring the
null value for my.empty, producing a completely different file than
what Config read:

  [my]
    empty
    enabled

Instead handle the writing differently to ensure the original format
is output.  New tests cases cover the expected behavior and return
values from accessor methods.

Change-Id: Id37379ce20cb27e3330923cf989444dd9f2bdd96
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryConfigTest.java
index 2cbfc682ca9dfd95713178e51c02cbac7ebe2f39..203b7c82514a3045da23cabcd98ecb9fef213734 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryConfigTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryConfigTest.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2007, Dave Watson <dwatson@mimvista.com>
- * Copyright (C) 2009, Google Inc.
+ * Copyright (C) 2009-2010, Google Inc.
  * Copyright (C) 2008, Marek Zawirski <marek.zawirski@gmail.com>
  * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
@@ -242,6 +242,35 @@ public class RepositoryConfigTest extends TestCase {
                }
        }
 
+       public void testBooleanWithNoValue() throws ConfigInvalidException {
+               Config c = parse("[my]\n\tempty\n");
+               assertEquals("", c.getString("my", null, "empty"));
+               assertEquals(1, c.getStringList("my", null, "empty").length);
+               assertEquals("", c.getStringList("my", null, "empty")[0]);
+               assertTrue(c.getBoolean("my", "empty", false));
+               assertEquals("[my]\n\tempty\n", c.toText());
+       }
+
+       public void testEmptyString() throws ConfigInvalidException {
+               Config c = parse("[my]\n\tempty =\n");
+               assertNull(c.getString("my", null, "empty"));
+
+               String[] values = c.getStringList("my", null, "empty");
+               assertNotNull(values);
+               assertEquals(1, values.length);
+               assertNull(values[0]);
+
+               // always matches the default, because its non-boolean
+               assertTrue(c.getBoolean("my", "empty", true));
+               assertFalse(c.getBoolean("my", "empty", false));
+
+               assertEquals("[my]\n\tempty =\n", c.toText());
+
+               c = new Config();
+               c.setStringList("my", null, "empty", Arrays.asList(values));
+               assertEquals("[my]\n\tempty =\n", c.toText());
+       }
+
        private void assertReadLong(long exp) throws ConfigInvalidException {
                assertReadLong(exp, String.valueOf(exp));
        }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
index 661371ca21c5b4f31e468b361b195e8d7eee2387..d66aa74c8e67188f6104edaf2119b28e446211b4 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
@@ -715,9 +715,10 @@ public class Config {
                                if (e.prefix == null || "".equals(e.prefix))
                                        out.append('\t');
                                out.append(e.name);
-                               if (e.value != null) {
-                                       if (MAGIC_EMPTY_VALUE != e.value) {
-                                               out.append(" = ");
+                               if (MAGIC_EMPTY_VALUE != e.value) {
+                                       out.append(" =");
+                                       if (e.value != null) {
+                                               out.append(' ');
                                                out.append(escapeValue(e.value));
                                        }
                                }