Fixes after review round

8 files changed, 52 insertions, 65 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Comparables.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Comparables.java
new file mode 100644
index 00000000000..b6fea7ad972
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Comparables.java
@@ -0,0 +1,24 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+package com.yahoo.vespa.hosted.node.admin.task.util.editor;
+
+/**
+ * @author hakon
+ */
+public class Comparables {
+    public static <T extends Comparable<T>> T min(T first, T second) {
+        if (first.compareTo(second) < 0) {
+            return first;
+        } else {
+            return second;
+        }
+    }
+
+    public static <T extends Comparable<T>> T max(T first, T second) {
+        if (first.compareTo(second) < 0) {
+            return second;
+        } else {
+            return first;
+        }
+    }
+}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Cursor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Cursor.java
index a07058ac575..85e98052bba 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Cursor.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Cursor.java
@@ -2,7 +2,6 @@
 
 package com.yahoo.vespa.hosted.node.admin.task.util.editor;
 
-import java.util.List;
 import java.util.Optional;
 import java.util.function.Function;
 import java.util.regex.Pattern;
@@ -59,7 +58,7 @@ public interface Cursor {
     Cursor write(String text);
     Cursor writeLine(String line);
     Cursor writeLines(String... lines);
-    Cursor writeLines(List<String> lines);
+    Cursor writeLines(Iterable<String> lines);
 
     Cursor writeNewline();
     Cursor writeNewlineAfter();
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/CursorImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/CursorImpl.java
index 35bd6f74aa1..c5f708d908c 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/CursorImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/CursorImpl.java
@@ -3,12 +3,14 @@
 package com.yahoo.vespa.hosted.node.admin.task.util.editor;
 
 import java.util.Arrays;
-import java.util.List;
 import java.util.Optional;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.regex.Pattern;
 
+import static com.yahoo.vespa.hosted.node.admin.task.util.editor.Comparables.max;
+import static com.yahoo.vespa.hosted.node.admin.task.util.editor.Comparables.min;
+
 /**
  * @author hakon
  */
@@ -65,13 +67,8 @@ public class CursorImpl implements Cursor {
     public String getTextTo(Mark mark) {
         validateMark(mark);
 
-        Position start = mark.position();
-        Position end = position;
-        if (start.isAfter(end)) {
-            Position tmp = start;
-            start = end;
-            end = tmp;
-        }
+        Position start = min(mark.position(), position);
+        Position end = max(mark.position(), position);
 
         return textBuffer.getSubstring(start, end);
     }
@@ -230,7 +227,7 @@ public class CursorImpl implements Cursor {
     }
 
     @Override
-    public Cursor writeLines(List<String> lines) {
+    public Cursor writeLines(Iterable<String> lines) {
         return writeLine(String.join("\n", lines));
     }
 
@@ -299,13 +296,9 @@ public class CursorImpl implements Cursor {
 
     @Override
     public Cursor deleteTo(Mark mark) {
-        Position start = mark.position();
-        Position end = position;
-        if (start.isAfter(end)) {
-            Position tmp = start;
-            start = end;
-            end = tmp;
-        }
+        validateMark(mark);
+        Position start = min(mark.position(), position);
+        Position end = max(mark.position(), position);
 
         textBuffer.delete(start, end);
         return this;
@@ -318,7 +311,6 @@ public class CursorImpl implements Cursor {
             return false;
         }
 
-        // position is unaffected by delete since position == match.get().startOfMatch()
         textBuffer.delete(match.get().startOfMatch(), match.get().endOfMatch());
         write(replacer.apply(match.get()));
         return true;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/FileEditor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/FileEditor.java
index 50d648d2eef..9896c86e286 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/FileEditor.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/FileEditor.java
@@ -34,7 +34,7 @@ public class FileEditor {
         return stringEditor.cursor();
     }
 
-    public void rereadFile() {
+    public void reloadFile() {
         fileText = path.readUtf8File();
         stringEditor.cursor().deleteAll().write(fileText);
         fileVersion = stringEditor.bufferVersion();
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Position.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Position.java
index c1e03a37a7d..f69b850ee91 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Position.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Position.java
@@ -15,8 +15,8 @@ public class Position implements Comparable<Position> {
     private static final Position START_POSITION = new Position(0, 0);
 
     private static final Comparator<Position> COMPARATOR = Comparator
-            .comparingInt((Position position) -> position.lineIndex())
-            .thenComparingInt((Position position) -> position.columnIndex());
+            .comparingInt(Position::lineIndex)
+            .thenComparingInt(Position::columnIndex);
 
     private final int lineIndex;
     private final int columnIndex;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextBuffer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextBuffer.java
index 8d7c7d8269a..1a224d5fab6 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextBuffer.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextBuffer.java
@@ -159,7 +159,7 @@ interface TextBuffer {
     }
 
     default Optional<Match> findForward(Position startPosition, Pattern pattern) {
-        for (Position position = startPosition;; position = getStartOfNextLine(position)) {
+        for (Position position = startPosition; ; position = getStartOfNextLine(position)) {
             String line = getLine(position);
             Matcher matcher = pattern.matcher(line);
             if (matcher.find(position.columnIndex())) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextBufferImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextBufferImpl.java
index 0c9d80db055..aa3eb04d2d9 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextBufferImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextBufferImpl.java
@@ -11,17 +11,17 @@ import static com.yahoo.vespa.hosted.node.admin.task.util.editor.TextUtil.splitS
  * @author hakon
  */
 public class TextBufferImpl implements TextBuffer {
+    /** Invariant: {@code size() >= 1}. An empty text buffer {@code => [""]} */
     private final LinkedList<String> lines = new LinkedList<>();
 
     private Version version = new Version();
 
     TextBufferImpl() {
-        // Invariant about always size() >= 0, and an empty text buffer => [""]
         lines.add("");
     }
 
     TextBufferImpl(String text) {
-        lines.add("");
+        this();
         write(getStartOfText(), text);
         // reset version
         version = new Version();
@@ -49,34 +49,26 @@ public class TextBufferImpl implements TextBuffer {
 
     @Override
     public Position write(Position position, String text) {
-        List<String> linesToInsert = splitString(text, true, false);
+        List<String> linesToInsert = new LinkedList<>(splitString(text, true, false));
         if (linesToInsert.isEmpty()) {
             return position;
         }
 
-        int lineIndex = position.lineIndex();
-
-        String prefix = getLinePrefix(position);
-        linesToInsert.set(0, prefix + linesToInsert.get(0));
-
-        // Insert the current prefix to the first line
-        int numberOfLinesToInsert = linesToInsert.size() - 1; // 1 will be set, the rest inserted
-        String prefixOfCursorAfterwards = linesToInsert.get(numberOfLinesToInsert);
+        // The position splits that line in two, and both prefix and suffix must be preserved
+        linesToInsert.set(0, getLinePrefix(position) + linesToInsert.get(0));
+        String lastLine = linesToInsert.get(linesToInsert.size() - 1);
+        int endColumnIndex = lastLine.length();
+        linesToInsert.set(linesToInsert.size() - 1, lastLine + getLineSuffix(position));
 
-        // Append the current suffix to the last line
-        String suffix = getLineSuffix(position);
-        String lastLine = prefixOfCursorAfterwards + suffix;
-        linesToInsert.set(numberOfLinesToInsert, lastLine);
-
-        // The first line is overwritten with set()
+        // Set the first line at lineIndex, insert the rest.
+        int lineIndex = position.lineIndex();
+        int endLineIndex = lineIndex + linesToInsert.size() - 1;
         lines.set(lineIndex, linesToInsert.remove(0));
-
-        // The following lines must be inserted
         lines.addAll(lineIndex + 1, linesToInsert);
 
         incrementVersion();
 
-        return new Position(lineIndex + linesToInsert.size(), prefixOfCursorAfterwards.length());
+        return new Position(endLineIndex, endColumnIndex);
     }
 
     @Override
@@ -104,12 +96,9 @@ public class TextBufferImpl implements TextBuffer {
     }
 
     private void deleteLines(int startIndex, int endIndex) {
-        int fromIndex = endIndex;
-        for (int toIndex = startIndex; fromIndex <= getMaxLineIndex(); ++toIndex, ++fromIndex) {
-            lines.set(toIndex, lines.get(fromIndex));
+        for (int i = startIndex; i < endIndex; ++i) {
+            lines.remove(startIndex);
         }
-
-        truncate(getMaxLineIndex() - (endIndex - startIndex));
     }
 
     private void truncate(int newMaxLineIndex) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextUtil.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextUtil.java
index 81bf24822dd..410ccbfc880 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextUtil.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/TextUtil.java
@@ -13,23 +13,6 @@ public class TextUtil {
     private TextUtil() {}
 
     /**
-     * Splits {@code text} by newline.
-     *
-     * @param text
-     * @param prune remove the last line if it is empty. Examples:
-     *              {@code "" -> []},
-     *              {@code "foo\n" -> "foo"},
-     *              {@code "foo\n\n" -> ["foo", ""]}. If false, these would return
-     *              {@code [""]},
-     *              {@code ["foo", ""]}, and
-     *              {@code ["foo", "", ""]}, respectively.
-     * @see #splitString(String, boolean, boolean)
-     */
-    public static List<String> splitString(String text, boolean prune) {
-        return splitString(text, prune, prune);
-    }
-
-    /**
      * Splits {@code text} by newline (LF {@code '\n'}).
      *
      * @param text the text to split into lines