From 96eb3ee3976e7e9e3e118851fa614cce8a1f7d88 Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Thu, 4 Jun 2015 11:35:24 -0700
Subject: [PATCH] Config: Distinguish between empty and null strings

The C git API and command line tools distinguish between a key having
the empty string as a value and no key being present in the config
file:

$ echo -e '[a]\nx =' > foo.config
$ git config -f foo.config a.x; echo $?

0
$ git config -f foo.config a.y; echo $?
1

Make JGit make the same distinction. This is in line with the current
Javadoc of getString, which claims to return "a String value from the
config, null if not found". It is more reasonable to interpret "x ="
in the above example as "found" rather than "missing".

We need to maintain the special handling of a key name with no "="
resolving to a boolean true, but "=" with an empty string is still not
a valid boolean.

Change-Id: If0dbb7470c524259de0b167148db87f81be2d04a
---
 .../tst/org/eclipse/jgit/lib/ConfigTest.java  | 71 +++++++++++++------
 .../src/org/eclipse/jgit/lib/Config.java      |  2 +-
 2 files changed, 50 insertions(+), 23 deletions(-)

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
index db31fd34c..73e8a041b 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
@@ -51,7 +51,6 @@ package org.eclipse.jgit.lib;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
@@ -502,27 +501,6 @@ public class ConfigTest {
 		assertEquals("[my]\n\tempty\n", c.toText());
 	}
 
-	@Test
-	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());
-	}
-
 	@Test
 	public void testUnsetBranchSection() throws ConfigInvalidException {
 		Config c = parse("" //
@@ -699,6 +677,55 @@ public class ConfigTest {
 		assertEquals("1", c.getString("a", null, "y"));
 	}
 
+	@Test
+	public void testExplicitlySetEmptyString() throws Exception {
+		Config c = new Config();
+		c.setString("a", null, "x", "0");
+		c.setString("a", null, "y", "");
+
+		assertEquals("0", c.getString("a", null, "x"));
+		assertEquals(0, c.getInt("a", null, "x", 1));
+
+		assertEquals("", c.getString("a", null, "y"));
+		assertArrayEquals(new String[]{""}, c.getStringList("a", null, "y"));
+		try {
+			c.getInt("a", null, "y", 1);
+		} catch (IllegalArgumentException e) {
+			assertEquals("Invalid integer value: a.y=", e.getMessage());
+		}
+
+		assertNull(c.getString("a", null, "z"));
+		assertArrayEquals(new String[]{}, c.getStringList("a", null, "z"));
+	}
+
+	@Test
+	public void testParsedEmptyString() throws Exception {
+		Config c = parse("[a]\n"
+				+ "x = 0\n"
+				+ "y =\n");
+
+		assertEquals("0", c.getString("a", null, "x"));
+		assertEquals(0, c.getInt("a", null, "x", 1));
+
+		assertEquals("", c.getString("a", null, "y"));
+		assertArrayEquals(new String[]{""}, c.getStringList("a", null, "y"));
+		try {
+			c.getInt("a", null, "y", 1);
+		} catch (IllegalArgumentException e) {
+			assertEquals("Invalid integer value: a.y=", e.getMessage());
+		}
+
+		assertNull(c.getString("a", null, "z"));
+		assertArrayEquals(new String[]{}, c.getStringList("a", null, "z"));
+	}
+
+	@Test
+	public void testSetStringListWithEmptyValue() throws Exception {
+		Config c = new Config();
+		c.setStringList("a", null, "x", Arrays.asList(""));
+		assertArrayEquals(new String[]{""}, c.getStringList("a", null, "x"));
+	}
+
 	private static 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 22337e813..fff82a32f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
@@ -1268,7 +1268,7 @@ public class Config {
 
 			value.append((char) c);
 		}
-		return value.length() > 0 ? value.toString() : null;
+		return value.toString();
 	}
 
 	/**