From 1b257fd426e296c8c161674cbf8cf8eab5d48b22 Mon Sep 17 00:00:00 2001 From: Preben Ingvaldsen Date: Wed, 1 Apr 2015 09:54:34 -0700 Subject: [PATCH] Address PR feedback and add new test Cleanup the indentText() method on ConfigNodeComplexValue as per PR feedback. Add a test for indentation when inserting a value with an include statement. --- .../typesafe/config/impl/ConfigNodeArray.java | 5 ++++ .../config/impl/ConfigNodeComplexValue.java | 30 +++++++++---------- .../config/impl/ConfigNodeConcatenation.java | 5 ++++ .../config/impl/ConfigNodeInclude.java | 7 +++++ .../config/impl/ConfigNodeObject.java | 5 ++++ .../typesafe/config/impl/ConfigNodeRoot.java | 5 ++++ .../config/impl/ConfigDocumentTest.scala | 9 ++++++ 7 files changed, 51 insertions(+), 15 deletions(-) diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigNodeArray.java b/config/src/main/java/com/typesafe/config/impl/ConfigNodeArray.java index 13c346aa..46500b52 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigNodeArray.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigNodeArray.java @@ -6,4 +6,9 @@ final class ConfigNodeArray extends ConfigNodeComplexValue { ConfigNodeArray(Collection children) { super(children); } + + @Override + protected ConfigNodeArray newNode(Collection nodes) { + return new ConfigNodeArray(nodes); + } } diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigNodeComplexValue.java b/config/src/main/java/com/typesafe/config/impl/ConfigNodeComplexValue.java index b0fca806..50d0e106 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigNodeComplexValue.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigNodeComplexValue.java @@ -28,25 +28,25 @@ abstract class ConfigNodeComplexValue extends AbstractConfigNodeValue { protected ConfigNodeComplexValue indentText(AbstractConfigNode indentation) { ArrayList childrenCopy = new ArrayList(children); for (int i = 0; i < childrenCopy.size(); i++) { - if (childrenCopy.get(i) instanceof ConfigNodeSingleToken && - Tokens.isNewline(((ConfigNodeSingleToken) childrenCopy.get(i)).token())) { + AbstractConfigNode child = childrenCopy.get(i); + if (child instanceof ConfigNodeSingleToken && + Tokens.isNewline(((ConfigNodeSingleToken) child).token())) { childrenCopy.add(i + 1, indentation); i++; - } else if (childrenCopy.get(i) instanceof ConfigNodeField) { - AbstractConfigNode value = ((ConfigNodeField) childrenCopy.get(i)).value(); - if (value instanceof ConfigNodeComplexValue) { - childrenCopy.set(i, ((ConfigNodeField) childrenCopy.get(i)).replaceValue(((ConfigNodeComplexValue) value).indentText(indentation))); + } else if (child instanceof ConfigNodeField) { + AbstractConfigNode value = ((ConfigNodeField) child).value(); + if (value instanceof ConfigNodeComplexValue && !(value instanceof ConfigNodeInclude)) { + childrenCopy.set(i, ((ConfigNodeField) child).replaceValue(((ConfigNodeComplexValue) value).indentText(indentation))); } - } else if (childrenCopy.get(i) instanceof ConfigNodeComplexValue) { - childrenCopy.set(i, ((ConfigNodeComplexValue) childrenCopy.get(i)).indentText(indentation)); + } else if (child instanceof ConfigNodeComplexValue && !(child instanceof ConfigNodeInclude)) { + childrenCopy.set(i, ((ConfigNodeComplexValue) child).indentText(indentation)); } } - if (this instanceof ConfigNodeArray) { - return new ConfigNodeArray(childrenCopy); - } else if (this instanceof ConfigNodeObject) { - return new ConfigNodeObject(childrenCopy); - } else { - return new ConfigNodeConcatenation(childrenCopy); - } + return newNode(childrenCopy); } + + // This method will just call into the object's constructor, but it's needed + // for use in the indentText() method so we can avoid a gross if/else statement + // checking the type of this + abstract ConfigNodeComplexValue newNode(Collection nodes); } diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigNodeConcatenation.java b/config/src/main/java/com/typesafe/config/impl/ConfigNodeConcatenation.java index b181a4dc..419cea6b 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigNodeConcatenation.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigNodeConcatenation.java @@ -6,4 +6,9 @@ final class ConfigNodeConcatenation extends ConfigNodeComplexValue { ConfigNodeConcatenation(Collection children) { super(children); } + + @Override + protected ConfigNodeConcatenation newNode(Collection nodes) { + return new ConfigNodeConcatenation(nodes); + } } diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigNodeInclude.java b/config/src/main/java/com/typesafe/config/impl/ConfigNodeInclude.java index 04b303ac..73ab3978 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigNodeInclude.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigNodeInclude.java @@ -1,5 +1,7 @@ package com.typesafe.config.impl; +import com.typesafe.config.ConfigException; + import java.util.Collection; final class ConfigNodeInclude extends ConfigNodeComplexValue { @@ -10,6 +12,11 @@ final class ConfigNodeInclude extends ConfigNodeComplexValue { this.kind = kind; } + @Override + protected ConfigNodeInclude newNode(Collection nodes) { + throw new ConfigException.BugOrBroken("Tried to indent an include node"); + } + protected ConfigIncludeKind kind() { return kind; } diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigNodeObject.java b/config/src/main/java/com/typesafe/config/impl/ConfigNodeObject.java index 17c5b291..b7a337bc 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigNodeObject.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigNodeObject.java @@ -10,6 +10,11 @@ final class ConfigNodeObject extends ConfigNodeComplexValue { super(children); } + @Override + protected ConfigNodeObject newNode(Collection nodes) { + return new ConfigNodeObject(nodes); + } + public boolean hasValue(Path desiredPath) { for (AbstractConfigNode node : children) { if (node instanceof ConfigNodeField) { diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigNodeRoot.java b/config/src/main/java/com/typesafe/config/impl/ConfigNodeRoot.java index 72ca612d..c84c16f5 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigNodeRoot.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigNodeRoot.java @@ -15,6 +15,11 @@ final class ConfigNodeRoot extends ConfigNodeComplexValue { this.origin = origin; } + @Override + protected ConfigNodeRoot newNode(Collection nodes) { + throw new ConfigException.BugOrBroken("Tried to indent the root object"); + } + protected ConfigNodeComplexValue value() { for (AbstractConfigNode node : children) { if (node instanceof ConfigNodeComplexValue) { diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala index b4a571ba..6460a455 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala @@ -426,4 +426,13 @@ class ConfigDocumentTest extends TestUtils { assertEquals("a {\n b {\n f : 10\n c : {\n d:e\n }\n }\n}", configDocument.setValue("a.b.c", "{\n d:e\n}").render()) } + @Test + def configDocumentIndentationValueWithIncludeTest { + val origText = "a {\n b {\n c : 22\n }\n}" + val configDocument = ConfigDocumentFactory.parseString(origText) + + assertEquals("a {\n b {\n c : 22\n d : {\n include \"foo\"\n e:f\n }\n }\n}", + configDocument.setValue("a.b.d", "{\n include \"foo\"\n e:f\n}").render()) + } + }