yaml: whitespace/comment fixes

format/yaml/src/main/java/org/spongepowered/configurate/yaml/MergeTag.java

@@ -0,0 +1,26 @@
+/*
+ * Configurate
+ * Copyright (C) zml and Configurate contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.spongepowered.configurate.yaml;
+
+final class MergeTag {
+
+    static final MergeTag INSTANCE = new MergeTag();
+
+    private MergeTag() {
+    }
+
+}

format/yaml/src/main/java/org/spongepowered/configurate/yaml/ScalarStyle.java

@@ -113,7 +113,7 @@ static ScalarStyle fromSnakeYaml(final DumperOptions.ScalarStyle style) {
     }
 
     static {
-        for (ScalarStyle style : values()) {
+        for (final ScalarStyle style : values()) {
             BY_SNAKE.put(style.snake, style);
         }
     }

format/yaml/src/main/java/org/spongepowered/configurate/yaml/Tag.java

@@ -69,7 +69,12 @@ public String toString(final String own) {
             this(tagUri, supportedTypes, pattern, null);
         }
 
-        Scalar(final URI tagUri, final Set<Class<? extends V>> supportedTypes, final @Nullable Pattern pattern, final @Nullable ScalarStyle preferredScalarStyle) {
+        Scalar(
+            final URI tagUri,
+            final Set<Class<? extends V>> supportedTypes,
+            final @Nullable Pattern pattern,
+            final @Nullable ScalarStyle preferredScalarStyle
+        ) {
             super(tagUri, supportedTypes);
             this.pattern = pattern;
             this.preferredScalarStyle = preferredScalarStyle;
@@ -89,7 +94,8 @@ public String toString(final String own) {
         }
 
         /**
-         * Get the preferred scalar style to use for this type, when none is specifically used.
+         * Get the preferred scalar style to use for this type, when none is
+         * specifically used.
          *
          * @return the preferred scalar style
          * @since 4.2.0

format/yaml/src/main/java/org/spongepowered/configurate/yaml/Yaml11Tags.java

@@ -73,15 +73,16 @@ public String toString(final byte[] own) {
      *
      * @implNote Canonically, these are y|n in YAML 1.1, but because YAML 1.2
      *     will only support true|false, we will treat those as the default
-     *     output format.
+     *     output format. We also modify the regex to only automatically match
+     *     true/false and on/off in order to avoid cases of confusion (such as
+     *     the classic Norway problem).
      * @see <a href="https://yaml.org/type/bool.html">tag:yaml.org,2002:bool</a>
      * @since 4.2.0
      */
     public static final Tag.Scalar<Boolean> BOOL = new Tag.Scalar<Boolean>(
         yamlOrg("bool"),
         UnmodifiableCollections.toSet(Boolean.class),
-        Pattern.compile("y|Y|yes|Yes|YES|n|N|no|No|NO"
-            + "|true|True|TRUE|false|False|FALSE"
+        Pattern.compile("true|True|TRUE|false|False|FALSE"
             + "|on|On|ON|off|Off|OFF")
     ) {
         private final Set<String> trues = UnmodifiableCollections.toSet(
@@ -189,13 +190,13 @@ public String toString(final Number own) {
         // used as map key, where the next node will be a reference that should be merged in to this node
 
         @Override
-        public Object fromString(final String input) throws ParsingException {
-            throw new ParsingException(ParsingException.UNKNOWN_POS, ParsingException.UNKNOWN_POS, null, "Merge keys are not yet implemented", null);
+        public Object fromString(final String input) {
+            return MergeTag.INSTANCE;
         }
 
         @Override
-        public String toString(final Object own) {
-            return own.toString();
+        public String toString(final Object own) throws ParsingException {
+            throw new ParsingException(ParsingException.UNKNOWN_POS, ParsingException.UNKNOWN_POS, null, "Merge keys cannot be serialized", null);
         }
     };
 

format/yaml/src/main/java/org/spongepowered/configurate/yaml/YamlConfigurationLoader.java

@@ -30,8 +30,8 @@
 import org.spongepowered.configurate.util.UnmodifiableCollections;
 import org.yaml.snakeyaml.DumperOptions;
 import org.yaml.snakeyaml.LoaderOptions;
-import org.yaml.snakeyaml.reader.StreamReader;
 import org.yaml.snakeyaml.events.DocumentStartEvent;
+import org.yaml.snakeyaml.reader.StreamReader;
 
 import java.io.BufferedReader;
 import java.io.Writer;
@@ -210,6 +210,8 @@ protected void populate(final LoaderOptionSource options) {
             if (declared != null) {
                 this.style = declared;
             }
+
+            this.enableComments = options.getBoolean(false, "yaml", "comments-enabled");
         }
 
         /**
@@ -317,7 +319,6 @@ public YamlConfigurationLoader build() {
     private final DumperOptions options;
     private final YamlVisitor visitor;
     private final @Nullable NodeStyle defaultNodeStyle;
-    private final boolean enableComments;
 
     private YamlConfigurationLoader(final Builder builder) {
         super(builder, new CommentHandler[] {CommentHandlers.HASH});
@@ -326,19 +327,18 @@ private YamlConfigurationLoader(final Builder builder) {
         opts.setDefaultFlowStyle(NodeStyle.asSnakeYaml(builder.nodeStyle()));
         opts.setProcessComments(builder.commentsEnabled());
         this.defaultNodeStyle = builder.nodeStyle();
-        this.enableComments = builder.commentsEnabled();
         this.options = opts;
         this.loader = new LoaderOptions()
             .setAcceptTabs(true)
             .setProcessComments(builder.commentsEnabled());
         this.loader.setCodePointLimit(Integer.MAX_VALUE);
-        this.visitor = new YamlVisitor(this.enableComments, true, Yaml11Tags.REPOSITORY);
+        this.visitor = new YamlVisitor(true, Yaml11Tags.REPOSITORY);
     }
 
     @Override
     protected void loadInternal(final CommentedConfigurationNode node, final BufferedReader reader) throws ParsingException {
         // Match the superclass implementation, except we substitute our own scanner implementation
-        final YamlParserComposer parser = new YamlParserComposer(new StreamReader(reader), this.loader, Yaml11Tags.REPOSITORY, this.enableComments);
+        final YamlParserComposer parser = new YamlParserComposer(new StreamReader(reader), this.loader, Yaml11Tags.REPOSITORY);
         parser.singleDocumentStream(node);
     }
 

format/yaml/src/main/java/org/spongepowered/configurate/yaml/YamlParserComposer.java

@@ -66,7 +66,6 @@ final class YamlParserComposer extends ParserImpl {
     private static final int FRAME_STACK_INCREMENT = 8;
 
     private @Nullable StringBuilder commentCollector;
-    private final boolean processComments;
     private final boolean stripLeadingCommentWhitespace = true;
     final Map<String, ConfigurationNode> aliases = new HashMap<>();
     final TagRepository tags;
@@ -75,9 +74,8 @@ final class YamlParserComposer extends ParserImpl {
     private Frame[] frames = new Frame[INITIAL_STACK_SIZE];
     private int framePointer = -1;
 
-    YamlParserComposer(final StreamReader reader, final LoaderOptions opts, final TagRepository tags, final boolean enableComments) {
+    YamlParserComposer(final StreamReader reader, final LoaderOptions opts, final TagRepository tags) {
         super(new ScannerImpl(reader, opts));
-        this.processComments = enableComments;
         this.tags = tags;
     }
 
@@ -226,6 +224,9 @@ static class Frame {
 
         static final int SUPPRESS_COMMENTS = 1; // whether to associate comment events with this node
         static final int SAVE_NODE = 1 << 1; // don't clear node when popping
+        static final int MERGE_REFERENCE_VALUE = 1 << 2; // when values have an anchor
+
+        static final int UNINHERITABLE_FLAGS = MERGE_REFERENCE_VALUE;
 
         @MonotonicNonNull ComposerState state;
 
@@ -236,13 +237,13 @@ static class Frame {
          * tag resolution.</p>
          */
         @Nullable Tag resolvedTag;
-        ConfigurationNode node;
+        @MonotonicNonNull ConfigurationNode node;
         int flags;
 
         void init(final ComposerState state, final Frame parent) {
             this.state = state;
             this.node = parent.node;
-            this.flags = parent.flags;
+            this.flags = parent.flags & ~UNINHERITABLE_FLAGS;
             this.resolvedTag = null;
         }
 
@@ -323,10 +324,6 @@ void applyComment(final @Nullable String comment, final ConfigurationNode node)
     }
 
     void collectComments() {
-        if (!this.processComments) {
-            return;
-        }
-
         while (this.peekEvent().is(Event.ID.Comment)) {
             final CommentEvent event = (CommentEvent) this.getEvent();
             if (event.getCommentType() != CommentType.BLANK_LINE) {
@@ -342,6 +339,10 @@ void collectComments() {
                 } else {
                     commentCollector.append(event.getValue());
                 }
+            } else if (this.commentCollector != null
+                && this.commentCollector.length() > 0
+                && this.peekEvent().is(Event.ID.Comment)) { // mid-comment blank line
+                this.commentCollector.append("\n");
             }
         }
     }
@@ -607,6 +608,11 @@ public Frame accept(final Frame head, final YamlParserComposer self) throws Pars
             if (keyHolder == null) {
                 throw new IllegalStateException("null keyHolder");
             }
+            // if merge key, set a flag on the next value state
+            if (keyHolder.ownHint(YamlConfigurationLoader.TAG) == Yaml11Tags.MERGE) {
+                head.addFlag(Frame.MERGE_REFERENCE_VALUE);
+            }
+
             final @Nullable Object key = keyHolder.raw();
             if (key == null) {
                 throw head.makeError(self.scanner.peekToken().getStartMark(), "'null' is not permitted as a mapping key", null);
@@ -683,8 +689,14 @@ private Alias() {
             if (target == null) {
                 throw head.makeError(event.getStartMark(), "Unknown anchor '" + event.getAnchor() + "'", null);
             }
-            head.node.from(target); // TODO: Reference node types
-            head.node.hint(YamlConfigurationLoader.ANCHOR_ID, null); // don't duplicate alias
+            final ConfigurationNode into;
+            if (head.hasFlag(Frame.MERGE_REFERENCE_VALUE)) {
+                into = head.node.parent();
+            } else {
+                into = head.node;
+            }
+            into.from(target); // TODO: Reference node types
+            into.hint(YamlConfigurationLoader.ANCHOR_ID, null); // don't duplicate alias
             return null;
         }
 

format/yaml/src/main/java/org/spongepowered/configurate/yaml/YamlVisitor.java

@@ -52,19 +52,17 @@ final class YamlVisitor implements ConfigurationVisitor<YamlVisitor.State, Void,
         null,
         null
     );
-    private static final CommentEvent COMMENT_BLANK_LINE = new CommentEvent(CommentType.BLOCK, "", null, null);
+    private static final CommentEvent COMMENT_BLANK_LINE = new CommentEvent(CommentType.BLANK_LINE, "", null, null);
     static final StreamStartEvent STREAM_START = new StreamStartEvent(null, null);
     static final StreamEndEvent STREAM_END = new StreamEndEvent(null, null);
     static final DocumentEndEvent DOCUMENT_END = new DocumentEndEvent(null, null, false);
     private static final SequenceEndEvent SEQUENCE_END = new SequenceEndEvent(null, null);
     private static final MappingEndEvent MAPPING_END = new MappingEndEvent(null, null);
 
     private final boolean shouldPadComments;
-    private final boolean enableComments;
     private final TagRepository tags;
 
-    YamlVisitor(final boolean enableComments, final boolean shouldPadComments, final TagRepository tags) {
-        this.enableComments = enableComments;
+    YamlVisitor(final boolean shouldPadComments, final TagRepository tags) {
         this.shouldPadComments = shouldPadComments;
         this.tags = tags;
     }
@@ -81,12 +79,17 @@ public void beginVisit(final ConfigurationNode node, final State state) {
 
     @Override
     public void enterNode(final ConfigurationNode node, final State state) throws ConfigurateException {
-        if (node instanceof CommentedConfigurationNodeIntermediary<@NonNull ?> && this.enableComments) {
+        if (node instanceof CommentedConfigurationNodeIntermediary<@NonNull ?> && state.options.isProcessComments()) {
             final @Nullable String comment = ((CommentedConfigurationNodeIntermediary<@NonNull ?>) node).comment();
             if (comment != null) {
-                if (this.shouldPadComments && node != state.start && !node.parent().isList()) {
-                    // todo: try and avoid emitting a blank line when we're the first element of a mapping?
-                    state.emit(WHITESPACE);
+                if (this.shouldPadComments && node != state.start) {
+                    if (!state.first) {
+                        if (!node.parent().isList()) {
+                            state.emit(WHITESPACE);
+                        }
+                    } else {
+                        state.first = false;
+                    }
                 }
                 for (final String line : COMMENT_SPLIT.split(comment, -1)) {
                     if (line.isEmpty()) {
@@ -111,13 +114,14 @@ public void enterNode(final ConfigurationNode node, final State state) throws Co
     @Override
     public void enterMappingNode(final ConfigurationNode node, final State state) throws ConfigurateException {
         final TagRepository.AnalyzedTag analysis = this.tags.analyze(node);
+        state.first = true;
         state.emit(new MappingStartEvent(
             this.anchor(node),
             analysis.actual().tagUri().toString(),
             analysis.implicit(),
             null,
             null,
-            NodeStyle.asSnakeYaml(this.determineStyle(node, state))
+            NodeStyle.asSnakeYaml(state.determineStyle(node))
         ));
     }
 
@@ -130,7 +134,7 @@ public void enterListNode(final ConfigurationNode node, final State state) throw
             analysis.implicit(),
             null,
             null,
-            NodeStyle.asSnakeYaml(this.determineStyle(node, state))
+            NodeStyle.asSnakeYaml(state.determineStyle(node))
         ));
     }
 
@@ -168,6 +172,7 @@ public void enterScalarNode(final ConfigurationNode node, final State state) thr
 
     @Override
     public void exitMappingNode(final ConfigurationNode node, final State state) throws ConfigurateException {
+        state.first = false; // only true if empty map
         state.emit(MAPPING_END);
     }
 
@@ -181,29 +186,32 @@ public Void endVisit(final State state) {
         return null;
     }
 
-    private @Nullable NodeStyle determineStyle(final ConfigurationNode node, final State state) {
-        // todo: some basic rules:
-        // - if a node has any children with comments, convert it to block style
-        // - when the default style is `AUTO` and `flowLevel` == 0,
-        final @Nullable NodeStyle style = node.hint(YamlConfigurationLoader.NODE_STYLE);
-        return style == null ? state.defaultStyle : style;
-    }
-
     private @Nullable String anchor(final ConfigurationNode node) {
         return node.hint(YamlConfigurationLoader.ANCHOR_ID);
     }
 
-    static class State {
+    static final class State {
         private final Emitter emit;
+        final DumperOptions options;
         @Nullable ConfigurationNode start;
         final @Nullable NodeStyle defaultStyle;
         ConfigurationNode mapKeyHolder;
+        boolean first; // reset to true at the beginning of each mapping node
 
         State(final DumperOptions options, final Writer writer, final @Nullable NodeStyle defaultStyle) {
             this.emit = new Emitter(writer, options);
+            this.options = options;
             this.defaultStyle = defaultStyle;
         }
 
+        @Nullable NodeStyle determineStyle(final ConfigurationNode node) {
+            // todo: some basic rules:
+            // - if a node has any children with comments, convert it to block style
+            // - when the default style is `AUTO` and `flowLevel` == 0,
+            final @Nullable NodeStyle style = node.hint(YamlConfigurationLoader.NODE_STYLE);
+            return style == null ? this.defaultStyle : style;
+        }
+
         public void emit(final Event event) throws ConfigurateException {
             try {
                 this.emit.emit(event);

format/yaml/src/test/groovy/org/spongepowered/configurate/yaml/IntegrationTests.groovy

@@ -28,7 +28,7 @@ class IntegrationTests implements YamlTest {
 
     @Test
     void testEssentialsXDefault() {
-        def input = this.class.getResourceAsStream("essx-example.yml").text
+        def input = this.class.getResourceAsStream("essx-example.yml").getText('utf-8')
         def node = parseString(input)
 
         def serialized = dump(node)
@@ -38,7 +38,7 @@ class IntegrationTests implements YamlTest {
 
     @Test
     void testEssentialsXLegacy() {
-        def input = this.class.getResourceAsStream("essx-legacy.yml").text
+        def input = this.class.getResourceAsStream("essx-legacy.yml").getText('utf-8')
         def node = parseString(input)
 
         def serialized = dump(node)
@@ -48,7 +48,7 @@ class IntegrationTests implements YamlTest {
 
     @Test
     void testMobCleaner() {
-        def input = this.class.getResourceAsStream("mobcleaner-example.yml").text
+        def input = this.class.getResourceAsStream("mobcleaner-example.yml").getText('utf-8')
         def node = parseString(input)
 
         def serialized = dump(node)