From d660fd800f0db08508a2ee84c9473c4f6a59398e Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Sat, 5 May 2018 18:11:48 +0200 Subject: Fixes after review round --- .../node/admin/task/util/editor/Comparables.java | 24 +++++++++++++ .../hosted/node/admin/task/util/editor/Cursor.java | 3 +- .../node/admin/task/util/editor/CursorImpl.java | 26 +++++---------- .../node/admin/task/util/editor/FileEditor.java | 2 +- .../node/admin/task/util/editor/Position.java | 4 +-- .../node/admin/task/util/editor/TextBuffer.java | 2 +- .../admin/task/util/editor/TextBufferImpl.java | 39 ++++++++-------------- .../node/admin/task/util/editor/TextUtil.java | 17 ---------- 8 files changed, 52 insertions(+), 65 deletions(-) create mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/editor/Comparables.java 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 min(T first, T second) { + if (first.compareTo(second) < 0) { + return first; + } else { + return second; + } + } + + public static > 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 lines); + Cursor writeLines(Iterable 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 lines) { + public Cursor writeLines(Iterable 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 { private static final Position START_POSITION = new Position(0, 0); private static final Comparator 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 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 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 linesToInsert = splitString(text, true, false); + List 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 @@ -12,23 +12,6 @@ import java.util.function.Consumer; 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 splitString(String text, boolean prune) { - return splitString(text, prune, prune); - } - /** * Splits {@code text} by newline (LF {@code '\n'}). * -- cgit v1.2.3