You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2018/09/26 05:14:19 UTC
[GitHub] geertjanw closed pull request #598: [NETBEANS-977] Improve text
layout in word wrap mode.
geertjanw closed pull request #598: [NETBEANS-977] Improve text layout in word wrap mode.
URL: https://github.com/apache/incubator-netbeans/pull/598
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/ide/editor.lib2/src/org/netbeans/api/editor/caret/EditorCaret.java b/ide/editor.lib2/src/org/netbeans/api/editor/caret/EditorCaret.java
index 64ffd85bd1..76a9cd9d55 100644
--- a/ide/editor.lib2/src/org/netbeans/api/editor/caret/EditorCaret.java
+++ b/ide/editor.lib2/src/org/netbeans/api/editor/caret/EditorCaret.java
@@ -25,7 +25,6 @@
import java.awt.Color;
import java.awt.Component;
import java.awt.Composite;
-import java.awt.Container;
import java.awt.Cursor;
import java.awt.Dimension;
import java.awt.Graphics;
@@ -71,7 +70,6 @@
import javax.swing.SwingUtilities;
import javax.swing.Timer;
import javax.swing.TransferHandler;
-import javax.swing.UIManager;
import javax.swing.event.ChangeEvent;
import javax.swing.event.ChangeListener;
import javax.swing.event.DocumentEvent;
@@ -87,7 +85,6 @@
import javax.swing.text.NavigationFilter;
import javax.swing.text.Position;
import javax.swing.text.StyleConstants;
-import javax.swing.text.Utilities;
import org.netbeans.api.annotations.common.CheckForNull;
import org.netbeans.api.annotations.common.NonNull;
import org.netbeans.api.annotations.common.NullAllowed;
@@ -1028,21 +1025,17 @@ public void deinstall(JTextComponent c) {
* For that, the nearest update must reposition scroller's viewrect so that the
* recomputed caretBounds is at the same
*/
- private synchronized void maybeSaveCaretOffset(JTextComponent c, Rectangle cbounds) {
- if (c.getClientProperty("editorcaret.updateRetainsVisibleOnce") == null || // NOI18N
+ private synchronized void maybeSaveCaretOffset(Rectangle cbounds) {
+ if (component.getClientProperty("editorcaret.updateRetainsVisibleOnce") == null || // NOI18N
lastCaretVisualOffset != -1) {
return;
}
- Component parent = c.getParent();
Rectangle editorRect;
- if (parent instanceof JLayeredPane) {
- parent = parent.getParent();
- }
- if (parent instanceof JViewport) {
- final JViewport viewport = (JViewport) parent;
+ final JViewport viewport = getViewport();
+ if (viewport != null) {
editorRect = viewport.getViewRect();
} else {
- Dimension size = c.getSize();
+ Dimension size = component.getSize();
editorRect = new Rectangle(0, 0, size.width, size.height);
}
if (cbounds.y >= editorRect.y && cbounds.y < (editorRect.y + editorRect.height)) {
@@ -1059,7 +1052,7 @@ private synchronized void maybeSaveCaretOffset(JTextComponent c, Rectangle cboun
@Override
public void paint(Graphics g) {
- JTextComponent c = component;
+ final JTextComponent c = component;
if (c == null || !isShowing()) {
return;
}
@@ -1109,7 +1102,7 @@ public void paint(Graphics g) {
Rectangle newCaretBounds = lvh.modelToViewBounds(dot, Position.Bias.Forward);
Rectangle oldBounds = caretItem.setCaretBoundsWithRepaint(newCaretBounds, c, "EditorCaret.paint()", i);
if (caretItem == lastCaret && oldBounds != null) {
- maybeSaveCaretOffset(c, oldBounds);
+ maybeSaveCaretOffset(oldBounds);
}
}
Rectangle caretBounds = caretItem.getCaretBounds();
@@ -1956,6 +1949,26 @@ private void dispatchUpdate(boolean forceInvokeLater) {
*/
private int lastCaretVisualOffset = -1;
+ private boolean isWrapping() {
+ // See o.n.modules.editor.lib2.view.DocumentViewOp.updateLineWrapType().
+ Object lwt = null;
+ if (component != null) {
+ lwt = component.getClientProperty(SimpleValueNames.TEXT_LINE_WRAP);
+ if (lwt == null) {
+ lwt = component.getDocument().getProperty(SimpleValueNames.TEXT_LINE_WRAP);
+ }
+ }
+ return (lwt instanceof String) && !"none".equals(lwt);
+ }
+
+ private JViewport getViewport() {
+ Component parent = component.getParent();
+ if (parent instanceof JLayeredPane) {
+ parent = parent.getParent();
+ }
+ return (parent instanceof JViewport) ? (JViewport) parent : null;
+ }
+
/**
* Update the caret's visual position.
* <br>
@@ -1973,13 +1986,9 @@ private void update(boolean calledFromPaint) {
if (c != null) {
boolean forceUpdate = c.getClientProperty("editorcaret.updateRetainsVisibleOnce") != null;
boolean log = LOG.isLoggable(Level.FINE);
- Component parent = c.getParent();
Rectangle editorRect;
- if (parent instanceof JLayeredPane) {
- parent = parent.getParent();
- }
- if (parent instanceof JViewport) {
- final JViewport viewport = (JViewport) parent;
+ final JViewport viewport = getViewport();
+ if (viewport != null) {
editorRect = viewport.getViewRect();
} else {
Dimension size = c.getSize();
@@ -1989,7 +1998,7 @@ private void update(boolean calledFromPaint) {
Rectangle cbounds = getLastCaretItem().getCaretBounds();
if (cbounds != null) {
// save relative position of the main caret
- maybeSaveCaretOffset(c, cbounds);
+ maybeSaveCaretOffset(cbounds);
}
}
if (!calledFromPaint && !c.isValid() /* && maintainVisible == null */) {
@@ -2028,27 +2037,40 @@ private void update(boolean calledFromPaint) {
}
if (caretBounds != null) {
Rectangle scrollBounds = new Rectangle(caretBounds); // Must possibly be cloned upon change
+ if (viewport != null && isWrapping()) {
+ /* When wrapping, only scroll to the right if the caret is
+ decisively outside the wrapped area (e.g. on a very long unbreakable
+ word). Otherwise, always scroll back to the left. When typing such
+ that the caret goes from the end of one wrap line to the next, the
+ new caret position might be one or more characters away from the
+ first character on the wrap line, so a regular
+ scroll-to-make-the-caret-visible would not do the job. */
+ if (scrollBounds.x <= viewport.getExtentSize().width) {
+ scrollBounds.x = 0;
+ scrollBounds.width = 1;
+ /* Avoid generating a drag-select as a result of the viewport
+ being automatically scrolled back to x=0 as a result of the user
+ clicking once to move the caret. */
+ if (viewport.getViewPosition().x > 0 && getDot() == getMark()) {
+ mouseState = MouseState.DEFAULT;
+ }
+ }
+ }
// Only scroll the view for the LAST caret to be visible
// For null old bounds (likely at begining of component displayment) ensure that a possible
// horizontal scrollbar would not hide the caret so enlarge the scroll bounds by hscrollbar height.
- if (oldCaretBounds == null) {
- Component viewport = c.getParent();
- if (viewport instanceof JLayeredPane) {
- viewport = viewport.getParent();
- }
- if (viewport instanceof JViewport) {
- Component scrollPane = viewport.getParent();
- if (scrollPane instanceof JScrollPane) {
- JScrollBar hScrollBar = ((JScrollPane) scrollPane).getHorizontalScrollBar();
- if (hScrollBar != null) {
- int hScrollBarHeight = hScrollBar.getPreferredSize().height;
- Dimension extentSize = ((JViewport) viewport).getExtentSize();
- // If the extent size is high enough then extend
- // the scroll region by extra vertical space
- if (extentSize.height >= caretBounds.height + hScrollBarHeight) {
- scrollBounds = new Rectangle(scrollBounds); // Clone
- scrollBounds.height += hScrollBarHeight;
- }
+ if (oldCaretBounds == null && viewport != null) {
+ Component scrollPane = viewport.getParent();
+ if (scrollPane instanceof JScrollPane) {
+ JScrollBar hScrollBar = ((JScrollPane) scrollPane).getHorizontalScrollBar();
+ if (hScrollBar != null) {
+ int hScrollBarHeight = hScrollBar.getPreferredSize().height;
+ Dimension extentSize = ((JViewport) viewport).getExtentSize();
+ // If the extent size is high enough then extend
+ // the scroll region by extra vertical space
+ if (extentSize.height >= caretBounds.height + hScrollBarHeight) {
+ scrollBounds = new Rectangle(scrollBounds); // Clone
+ scrollBounds.height += hScrollBarHeight;
}
}
}
@@ -2586,12 +2608,9 @@ public void actionPerformed(ActionEvent e) { // Blinker timer fired
// and if it's fired the caret's bounds will be checked whether
// they intersect with the horizontal scrollbar
// and if so the view will be scrolled.
- Container parent = component.getParent();
- if(parent instanceof JLayeredPane) {
- parent = parent.getParent();
- }
- if (parent instanceof JViewport) {
- parent = parent.getParent(); // parent of viewport
+ final JViewport viewport = getViewport();
+ if (viewport != null) {
+ Component parent = viewport.getParent();
if (parent instanceof JScrollPane) {
JScrollPane scrollPane = (JScrollPane) parent;
JScrollBar hScrollBar = scrollPane.getHorizontalScrollBar();
diff --git a/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/CharSequenceCharacterIterator.java b/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/CharSequenceCharacterIterator.java
new file mode 100644
index 0000000000..e4726efd54
--- /dev/null
+++ b/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/CharSequenceCharacterIterator.java
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.netbeans.modules.editor.lib2.view;
+
+import java.text.CharacterIterator;
+
+/* This class was written from scratch. I considered using org.apache.pivot.text.CharacterIterator
+from the Apache Pivot project, but it had bugs in the next() and previous() methods (trying to use
+CharacterIterator.DONE = 65535 as an index). */
+/**
+ * Adapter class for providing a {@link CharacterIterator} over a {@link CharSequence} without
+ * making a copy of the entire underlying string.
+ *
+ * @author Eirik Bakke (ebakke@ultorg.com)
+ */
+class CharSequenceCharacterIterator implements CharacterIterator {
+ private final CharSequence charSequence;
+ private int index;
+
+ public CharSequenceCharacterIterator(CharSequence charSequence) {
+ if (charSequence == null) {
+ throw new NullPointerException();
+ }
+ this.charSequence = charSequence;
+ }
+
+ @Override
+ public int getIndex() {
+ return index;
+ }
+
+ @Override
+ public char setIndex(int position) {
+ if (position < getBeginIndex() || position > getEndIndex()) {
+ throw new IllegalArgumentException();
+ }
+ this.index = position;
+ return current();
+ }
+
+ @Override
+ public char current() {
+ return index == getEndIndex() ? CharacterIterator.DONE : charSequence.charAt(index);
+ }
+
+ @Override
+ public char first() {
+ return setIndex(getBeginIndex());
+ }
+
+ @Override
+ public char last() {
+ final int endIndex = getEndIndex();
+ return setIndex(charSequence.length() == 0 ? endIndex : (endIndex - 1));
+ }
+
+ @Override
+ public char next() {
+ if (index < getEndIndex()) {
+ index++;
+ }
+ return current();
+ }
+
+ @Override
+ public char previous() {
+ if (index > getBeginIndex()) {
+ index--;
+ return current();
+ } else {
+ return CharacterIterator.DONE;
+ }
+ }
+
+ @Override
+ public int getBeginIndex() {
+ return 0;
+ }
+
+ @Override
+ public int getEndIndex() {
+ return charSequence.length();
+ }
+
+ @Override
+ public Object clone() {
+ CharacterIterator ret = new CharSequenceCharacterIterator(charSequence);
+ ret.setIndex(index);
+ return ret;
+ }
+}
diff --git a/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/DocumentViewOp.java b/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/DocumentViewOp.java
index b95c60d218..a226a303e8 100644
--- a/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/DocumentViewOp.java
+++ b/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/DocumentViewOp.java
@@ -1227,10 +1227,12 @@ float getAvailableWidth() {
setStatusBits(AVAILABLE_WIDTH_VALID);
availableWidth = Integer.MAX_VALUE;
renderWrapWidth = availableWidth;
- TextLayout lineContTextLayout = getLineContinuationCharTextLayout();
- if (lineContTextLayout != null && (getLineWrapType() != LineWrapType.NONE)) {
- availableWidth = Math.max(getVisibleRect().width, 4 * getDefaultCharWidth() + lineContTextLayout.getAdvance());
- renderWrapWidth = availableWidth - lineContTextLayout.getAdvance();
+ if (getLineWrapType() != LineWrapType.NONE) {
+ final TextLayout lineContTextLayout = getLineContinuationCharTextLayout();
+ final float lineContTextLayoutAdvance =
+ lineContTextLayout == null ? 0f : lineContTextLayout.getAdvance();
+ availableWidth = Math.max(getVisibleRect().width, 4 * getDefaultCharWidth() + lineContTextLayoutAdvance);
+ renderWrapWidth = availableWidth - lineContTextLayoutAdvance;
}
}
return availableWidth;
@@ -1343,7 +1345,17 @@ TextLayout getTabCharTextLayout(double availableWidth) {
return ret;
}
+ /**
+ * @return will be null if the line continuation character should not be shown
+ */
TextLayout getLineContinuationCharTextLayout() {
+ /* The line continuation character is used to show that a line is automatically being
+ broken into multiple wrap lines via the line wrap feature. This causes a lot of visual
+ clutter, and always takes up an extra character of horizontal space, so don't show it by
+ default. The same information is communicated by the line numbers in the left-hand side of
+ the editor anyway. */
+ if (!docView.op.isNonPrintableCharactersVisible())
+ return null;
if (lineContinuationTextLayout == null) {
char lineContinuationChar = LINE_CONTINUATION;
if (!defaultFont.canDisplay(lineContinuationChar)) {
diff --git a/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/HighlightsViewUtils.java b/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/HighlightsViewUtils.java
index 6666a42658..f13c6e7851 100644
--- a/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/HighlightsViewUtils.java
+++ b/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/HighlightsViewUtils.java
@@ -28,8 +28,9 @@
import java.awt.font.TextHitInfo;
import java.awt.font.TextLayout;
import java.awt.geom.Rectangle2D;
+import java.text.BreakIterator;
+import java.util.Locale;
import java.util.logging.Level;
-import java.util.logging.Logger;
import javax.swing.text.AttributeSet;
import javax.swing.text.Caret;
import javax.swing.text.JTextComponent;
@@ -714,52 +715,30 @@ static View breakView(int axis, int breakPartStartOffset, float x, float len,
}
}
- // Now perform corrections if wrapping at word boundaries is required
- // Currently a simple impl that checks adjacent char(s) in backward direction
- // is used. Consider BreakIterator etc. if requested.
- // If break is inside a word then check for word boundary in backward direction.
- // If none is found then go forward to find a word break if possible.
int breakPartEndOffset = partStartOffset + hitInfo.getCharIndex();
if (breakPartEndOffset > breakPartStartOffset) {
+ // Now perform corrections if wrapping at word boundaries is required
if (docView.op.getLineWrapType() == LineWrapType.WORD_BOUND) {
- CharSequence docText = DocumentUtilities.getText(docView.getDocument());
- if (breakPartEndOffset > breakPartStartOffset) {
- boolean searchNonLetterForward = false;
- char ch = docText.charAt(breakPartEndOffset - 1);
- // [TODO] Check surrogates
- if (Character.isLetterOrDigit(ch)) {
- if (breakPartEndOffset < docText.length() &&
- Character.isLetterOrDigit(docText.charAt(breakPartEndOffset)))
- {
- // Inside word
- // Attempt to go back and search non-letter
- int offset = breakPartEndOffset - 1;
- while (offset >= breakPartStartOffset && Character.isLetterOrDigit(docText.charAt(offset))) {
- offset--;
- }
- offset++;
- if (offset == breakPartStartOffset) {
- searchNonLetterForward = true;
- } else { // move the break offset back
- breakPartEndOffset = offset;
- }
- }
- }
- if (searchNonLetterForward) {
- breakPartEndOffset++; // char at breakPartEndOffset already checked
- while (breakPartEndOffset < partStartOffset + partLength &&
- Character.isLetterOrDigit(docText.charAt(breakPartEndOffset)))
- {
- breakPartEndOffset++;
- }
- }
- }
+ CharSequence paragraph = DocumentUtilities.getText(docView.getDocument())
+ .subSequence(breakPartStartOffset, partStartOffset + partLength);
+ /* Don't enable allowWhitespaceBeyondEnd if we are printing the line
+ continuation character
+ (see DocumentViewOp.getLineContinuationCharTextLayout), since the latter
+ would usually then end up beyond the edge of the editor viewport. */
+ boolean allowWhitespaceBeyondEnd =
+ !docView.op.isNonPrintableCharactersVisible();
+ breakPartEndOffset = adjustBreakOffsetToWord(paragraph,
+ breakPartEndOffset - breakPartStartOffset,
+ allowWhitespaceBeyondEnd) + breakPartStartOffset;
}
}
// Length must be > 0; BTW TextLayout can't be constructed with empty string.
- boolean breakFailed = (breakPartEndOffset - breakPartStartOffset == 0) ||
- (breakPartEndOffset - breakPartStartOffset >= partLength);
+ boolean doNotBreak =
+ // No need to split the line in two if the first part would be empty.
+ (breakPartEndOffset - breakPartStartOffset == 0) ||
+ // No need to split the line in two if the second part would be empty.
+ (breakPartEndOffset - breakPartStartOffset >= partLength);
// if (ViewHierarchyImpl.BUILD_LOG.isLoggable(Level.FINE)) {
// ViewHierarchyImpl.BUILD_LOG.fine("HV.breakView(): <" + partStartOffset + // NOI18N
// "," + (partStartOffset+partLength) + // NOI18N
@@ -767,7 +746,7 @@ static View breakView(int axis, int breakPartStartOffset, float x, float len,
// ">, x=" + x + ", len=" + len + // NOI18N
// ", charIndexX=" + breakCharIndexX + "\n"); // NOI18N
// }
- if (breakFailed) {
+ if (doNotBreak) {
return null;
}
return new HighlightsViewPart(fullView, breakPartStartOffset - fullViewStartOffset,
@@ -777,4 +756,90 @@ static View breakView(int axis, int breakPartStartOffset, float x, float len,
return null;
}
+ // Package-private for testing.
+ /**
+ * Calculate the position at which to break a line in a paragraph. A break offset of X means
+ * that the character with index (X-1) in {@code paragraph} will be the last one on the physical
+ * line.
+ *
+ * <p>The current implementation avoids creating lines with leading whitespace (when words are
+ * separated by at most one whitespace character), allows lines to be broken after hyphens, and,
+ * if {@code allowWhitespaceBeyondEnd} is true, allows one whitespace character to extend beyond
+ * the preferred break width to make use of all available horizontal space. Very long
+ * unbreakable words may extend beyond the preferred break offset regardless of the setting of
+ * {@code allowWhitespaceBeyondEnd}.
+ *
+ * <p>It was previously considered to allow an arbitrary number of whitespace characters to
+ * trail off the end of each wrap line, rather than just one. In the end, it turned out to be
+ * better to limit this to just one character, as this conveniently avoids the need to ever
+ * position the visual text caret outside the word-wrapped editor viewport (except in cases of
+ * very long unbreakable words).
+ *
+ * @param paragraph a long line of text to be broken, i.e. a paragraph, or the remainder of a
+ * paragraph if some of its initial lines of wrapped text have already been laid out
+ * @param preferredMaximumBreakOffset the preferred maximum break offset
+ * @param allowWhitespaceBeyondEnd if true, allow one whitespace character to extend beyond
+ * {@code preferredMaximumBreakOffset} even when this could be avoided by choosing a
+ * smaller break offset
+ */
+ static int adjustBreakOffsetToWord(CharSequence paragraph,
+ final int preferredMaximumBreakOffset, boolean allowWhitespaceBeyondEnd)
+ {
+ if (preferredMaximumBreakOffset < 0) {
+ throw new IllegalArgumentException();
+ }
+ if (preferredMaximumBreakOffset > paragraph.length()) {
+ throw new IllegalArgumentException();
+ }
+ /* BreakIterator.getLineInstance already seems to have a cache; creating a new instance here
+ is just the cost of BreakIterator.clone(). So don't bother trying to cache the BreakIterator
+ here. */
+ BreakIterator bi = BreakIterator.getLineInstance(Locale.US);
+ /* Use CharSequenceCharacterIterator to avoid copying the entire paragraph string every
+ time. */
+ bi.setText(new CharSequenceCharacterIterator(paragraph));
+
+ int ret;
+ if (preferredMaximumBreakOffset == 0) {
+ // Skip forward to next boundary.
+ ret = 0;
+ } else if (
+ allowWhitespaceBeyondEnd && preferredMaximumBreakOffset < paragraph.length() &&
+ Character.isWhitespace(paragraph.charAt(preferredMaximumBreakOffset)))
+ {
+ // Allow one whitespace character to extend beyond the preferred break offset.
+ return preferredMaximumBreakOffset + 1;
+ } else {
+ // Skip backwards to previous boundary.
+ ret = bi.isBoundary(preferredMaximumBreakOffset)
+ ? preferredMaximumBreakOffset
+ : bi.preceding(preferredMaximumBreakOffset);
+ if (ret == BreakIterator.DONE) {
+ return preferredMaximumBreakOffset;
+ }
+ }
+ if (ret == 0) {
+ // Skip forward to next boundary (for words longer than the preferred break offset).
+ ret = preferredMaximumBreakOffset > 0 && bi.isBoundary(preferredMaximumBreakOffset)
+ ? preferredMaximumBreakOffset
+ : bi.following(preferredMaximumBreakOffset);
+ if (ret == BreakIterator.DONE) {
+ ret = preferredMaximumBreakOffset;
+ }
+ /* The line-based break iterator will include whitespace trailing a word as well. Strip
+ this off so we can apply our own policy here. */
+ int retBeforeTrim = ret;
+ while (ret > preferredMaximumBreakOffset &&
+ Character.isWhitespace(paragraph.charAt(ret - 1)))
+ {
+ ret--;
+ }
+ /* If allowWhitespaceBeyondEnd is true, allow at most one whitespace character to trail
+ the word at the end. */
+ if ((allowWhitespaceBeyondEnd || ret == 0) && retBeforeTrim > ret) {
+ ret++;
+ }
+ }
+ return ret;
+ }
}
diff --git a/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/WrapInfo.java b/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/WrapInfo.java
index 0446647998..9e48bd56ca 100644
--- a/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/WrapInfo.java
+++ b/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/WrapInfo.java
@@ -122,7 +122,7 @@ void paintWrapLines(ParagraphViewChildren children, ParagraphView pView,
allocBounds.x += endPart.width;
}
// Paint wrap mark
- if (i != lastWrapLineIndex) { // but not on last wrap line
+ if (lineContinuationTextLayout != null && i != lastWrapLineIndex) { // but not on last wrap line
PaintState paintState = PaintState.save(g);
try {
ViewUtils.applyForegroundColor(g, null, textComponent);
diff --git a/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/WrapInfoUpdater.java b/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/WrapInfoUpdater.java
index 497873f69e..335d4382ee 100644
--- a/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/WrapInfoUpdater.java
+++ b/ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/WrapInfoUpdater.java
@@ -24,6 +24,7 @@
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
+import javax.swing.text.BadLocationException;
import javax.swing.text.View;
import org.netbeans.lib.editor.util.swing.DocumentUtilities;
@@ -101,14 +102,16 @@ void initWrapInfo() {
wrapTypeWords = (docView.op.getLineWrapType() == LineWrapType.WORD_BOUND);
float visibleWidth = docView.op.getVisibleRect().width;
TextLayout lineContinuationTextLayout = docView.op.getLineContinuationCharTextLayout();
+ final float lineContTextLayoutAdvance =
+ lineContinuationTextLayout == null ? 0f : lineContinuationTextLayout.getAdvance();
// Make reasonable minimum width so that the number of visual lines does not double suddenly
// when user would minimize the width too much. Also have enough space for line continuation mark
- availableWidth = Math.max(visibleWidth - lineContinuationTextLayout.getAdvance(),
+ availableWidth = Math.max(visibleWidth - lineContTextLayoutAdvance,
docView.op.getDefaultCharWidth() * 4);
logMsgBuilder = LOG.isLoggable(Level.FINE) ? new StringBuilder(100) : null;
if (logMsgBuilder != null) {
logMsgBuilder.append("Building wrapLines: availWidth=").append(availableWidth); // NOI18N
- logMsgBuilder.append(", lineContCharWidth=").append(lineContinuationTextLayout.getAdvance()); // NOI18N
+ logMsgBuilder.append(", lineContCharWidth=").append(lineContTextLayoutAdvance); // NOI18N
logMsgBuilder.append("\n"); // NOI18N
}
try {
@@ -131,7 +134,7 @@ void initWrapInfo() {
WordInfo wordInfo = getWordInfo(viewOrPartStartOffset, wrapLineStartOffset);
if (wordInfo != null) {
// Attempt to break the view (at word boundary) so that it fits.
- ViewSplit split = breakView(viewOrPart, false);
+ ViewSplit split = breakView(viewOrPart, true);
if (split != null) {
addPart(split.startPart);
finishWrapLine();
@@ -176,18 +179,49 @@ void initWrapInfo() {
}
if (regularBreak) {
- ViewSplit split = breakView(viewOrPart, false);
+ /* Use allowWider=true here, so that long words are allowed to extend beyond
+ the preferred wrap width. Turning it off would just give up on breaking
+ entirely for the rest of the paragraph, yielding an even longer physical
+ line. */
+ ViewSplit split = breakView(viewOrPart, true);
if (split != null) {
addPart(split.startPart);
viewOrPart = split.endPart;
- finishWrapLine();
} else { // break failed
if (!wrapLineNonEmpty) {
addViewOrPart(viewOrPart);
viewOrPart = fetchNextView();
}
- finishWrapLine();
}
+ /* Keep the NewlineView that follows each paragraph together with the
+ paragraph's last wrap line. Otherwise, the NewlineView might wrap to the
+ next physical line if the last wrap line of the paragraph happens to be
+ exactly as long as the availableWidth. This would make the text caret, if
+ positioned at the end of a paragraph, end up being visually positioned on
+ the beginning of the next line instead of at the end of the current one. */
+ if (viewOrPart != null && viewOrPart.view instanceof NewlineView) {
+ /* One exception: If the wrap line ends with a space, it's actually
+ better to allow the NewlineView, and thus the text caret, to wrap to the
+ next physical line, since this is where the user's next typed character
+ will end up. This also avoids the need to position the caret outside the
+ viewport in a few cases (due to the text wrapping policy of allowing
+ whitespace characters at the end of each wrap line to extend beyond the
+ preferred wrap width). */
+ boolean wrapLineEndsWithSpace = false;
+ try {
+ int newlineOffset = viewOrPart.view.getStartOffset();
+ wrapLineEndsWithSpace = newlineOffset > 0 &&
+ Character.isWhitespace(viewOrPart.view.getDocument()
+ .getText(newlineOffset - 1, 1).charAt(0));
+ } catch (BadLocationException e) {
+ // Ignore.
+ }
+ if (!wrapLineEndsWithSpace) {
+ addViewOrPart(viewOrPart);
+ viewOrPart = fetchNextView();
+ }
+ }
+ finishWrapLine();
}
}
} while (childIndex < pView.getViewCount());
@@ -601,6 +635,10 @@ int wordStartOffset() {
return wordStartOffset;
}
+ @Override
+ public String toString() {
+ return "WordInfo(" + wordStartOffset() + ", " + wordEndOffset() + ")";
+ }
}
diff --git a/ide/editor.lib2/test/unit/src/org/netbeans/modules/editor/lib2/view/AdjustBreakOffsetToWordTest.java b/ide/editor.lib2/test/unit/src/org/netbeans/modules/editor/lib2/view/AdjustBreakOffsetToWordTest.java
new file mode 100644
index 0000000000..2787530931
--- /dev/null
+++ b/ide/editor.lib2/test/unit/src/org/netbeans/modules/editor/lib2/view/AdjustBreakOffsetToWordTest.java
@@ -0,0 +1,284 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.netbeans.modules.editor.lib2.view;
+
+import java.util.Objects;
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Test for {@link HighlightsViewUtils#adjustBreakOffsetToWord(CharSequence,int,boolean)}.
+ */
+public class AdjustBreakOffsetToWordTest {
+ private void testOne(String original, String expected, boolean allowWhitespaceBeyondEnd) {
+ TextWithCaret originalTwC = TextWithCaret.fromEncoded(original);
+ TextWithCaret expectedTwC = TextWithCaret.fromEncoded(expected);
+ TextWithCaret actualTwC = new TextWithCaret(originalTwC.text,
+ HighlightsViewUtils.adjustBreakOffsetToWord(originalTwC.text, originalTwC.caret,
+ allowWhitespaceBeyondEnd));
+ Assert.assertEquals(expectedTwC, actualTwC);
+ }
+
+ private void testOne(String original, String expected) {
+ testOne(original, expected, false);
+ testOne(original, expected, true);
+ }
+
+ private void testOne(String original, String
+ expectedDisallowWhitespaceBeyondEnd, String expectedAllowWhitespaceBeyondEnd)
+ {
+ testOne(original, expectedDisallowWhitespaceBeyondEnd, false);
+ testOne(original, expectedAllowWhitespaceBeyondEnd, true);
+ }
+
+ @Test
+ public void testLongWord() {
+ // Forward-skipping is necessary.
+ testOne("|this is a test", "this| is a test", "this |is a test");
+ testOne("t|his is a test", "this| is a test", "this |is a test");
+ testOne("th|is is a test", "this| is a test", "this |is a test");
+ testOne("thi|s is a test", "this| is a test", "this |is a test");
+ testOne("this| is a test", "this| is a test", "this |is a test");
+ }
+
+ @Test
+ public void testBasic() {
+ // Forward-skipping is not necessary.
+ testOne("this |is a test", "this |is a test");
+ testOne("this i|s a test", "this |is a test");
+ /* If !allowWhitespaceBeyondEnd, break must backtrack to before "is" to avoid the new line
+ starting with a whitespace. */
+ testOne("this is| a test", "this |is a test", "this is |a test");
+ testOne("this is |a test", "this is |a test");
+ testOne("this is a| test", "this is |a test", "this is a |test");
+ testOne("this is a |test", "this is a |test");
+ testOne("this is a t|est", "this is a |test");
+ testOne("this is a te|st", "this is a |test");
+ testOne("this is a tes|t", "this is a |test");
+ testOne("this is a test|", "this is a test|");
+ }
+
+ @Test
+ public void testDashInFirstWord() {
+ testOne("|foo-test bar is", "foo-|test bar is");
+ testOne("f|oo-test bar is", "foo-|test bar is");
+ testOne("fo|o-test bar is", "foo-|test bar is");
+ testOne("foo|-test bar is", "foo-|test bar is");
+ testOne("foo-|test bar is", "foo-|test bar is");
+ testOne("foo-t|est bar is", "foo-|test bar is");
+ testOne("foo-te|st bar is", "foo-|test bar is");
+ testOne("foo-tes|t bar is", "foo-|test bar is");
+ testOne("foo-test| bar is", "foo-|test bar is", "foo-test |bar is");
+ testOne("foo-test |bar is", "foo-test |bar is");
+ testOne("foo-test b|ar is", "foo-test |bar is");
+ }
+
+ @Test
+ public void testDashInNonFirstWord() {
+ testOne("this is| foo-test bar", "this |is foo-test bar", "this is |foo-test bar");
+ testOne("this is |foo-test bar", "this is |foo-test bar");
+ testOne("this is f|oo-test bar", "this is |foo-test bar");
+ testOne("this is fo|o-test bar", "this is |foo-test bar");
+ testOne("this is foo|-test bar", "this is |foo-test bar");
+ testOne("this is foo-|test bar", "this is foo-|test bar");
+ testOne("this is foo-t|est bar", "this is foo-|test bar");
+ testOne("this is foo-te|st bar", "this is foo-|test bar");
+ testOne("this is foo-tes|t bar", "this is foo-|test bar");
+ testOne("this is foo-test| bar", "this is foo-|test bar", "this is foo-test |bar");
+ testOne("this is foo-test |bar", "this is foo-test |bar");
+ testOne("this is foo-test b|ar", "this is foo-test |bar");
+ }
+
+ @Test
+ public void testJustWhitespace() {
+ testOne("|", "|");
+ testOne("| ", " |");
+ testOne(" |", " |");
+ testOne("| ", " | ");
+ testOne(" | ", " | ", " |");
+ testOne(" |", " |");
+ testOne("| ", " | ");
+ testOne(" | ", " | ", " | ");
+ testOne(" | ", " | ", " |");
+ testOne(" |", " |");
+ }
+
+ @Test
+ public void testTrailingNewline() {
+ // A newline character should just be treated like any other whitespace here.
+ testOne("|this is a test\n", "this| is a test\n", "this |is a test\n");
+ testOne("t|his is a test\n", "this| is a test\n", "this |is a test\n");
+ testOne("th|is is a test\n", "this| is a test\n", "this |is a test\n");
+ testOne("thi|s is a test\n", "this| is a test\n", "this |is a test\n");
+ testOne("this| is a test\n", "this| is a test\n", "this |is a test\n");
+ testOne("this |is a test\n", "this |is a test\n");
+ testOne("this i|s a test\n", "this |is a test\n");
+ testOne("this is| a test\n", "this |is a test\n", "this is |a test\n");
+ testOne("this is |a test\n", "this is |a test\n");
+ testOne("this is a| test\n", "this is |a test\n", "this is a |test\n");
+ testOne("this is a |test\n", "this is a |test\n");
+ testOne("this is a t|est\n", "this is a |test\n");
+ testOne("this is a te|st\n", "this is a |test\n");
+ testOne("this is a tes|t\n", "this is a |test\n");
+ testOne("this is a test|\n", "this is a |test\n", "this is a test\n|");
+ testOne("this is a test\n|", "this is a test\n|");
+ }
+
+ @Test
+ public void testSpacesBetween() {
+ // Multiple whitespace characters between first and second word.
+ testOne("th|is is a test", "this| is a test", "this | is a test");
+ testOne("thi|s is a test", "this| is a test", "this | is a test");
+ testOne("this| is a test", "this| is a test", "this | is a test");
+ testOne("this | is a test", "this | is a test", "this | is a test");
+ testOne("this | is a test", "this | is a test", "this |is a test");
+ testOne("this |is a test", "this |is a test");
+ testOne("this i|s a test", "this |is a test");
+ testOne("this is| a test", "this |is a test", "this is |a test");
+ testOne("this is |a test", "this is |a test");
+ // Multiple whitespace characters between words not including the first word.
+ testOne("this |is aaaa test", "this |is aaaa test");
+ testOne("this i|s aaaa test", "this |is aaaa test");
+ testOne("this is| aaaa test", "this |is aaaa test", "this is | aaaa test");
+ testOne("this is | aaaa test", "this |is aaaa test", "this is | aaaa test");
+ testOne("this is | aaaa test", "this |is aaaa test", "this is |aaaa test");
+ testOne("this is |aaaa test", "this is |aaaa test");
+ testOne("this is a|aaa test", "this is |aaaa test");
+ testOne("this is aa|aa test", "this is |aaaa test");
+ testOne("this is aaa|a test", "this is |aaaa test");
+ testOne("this is aaaa| test", "this is |aaaa test", "this is aaaa |test");
+ testOne("this is aaaa |test", "this is aaaa |test");
+ }
+
+ @Test
+ public void testTrailingSpaces() {
+ // One trailing whitespace character.
+ testOne("this is a test| ", "this is a |test ", "this is a test |");
+ testOne("this is a test |", "this is a test |");
+ // Two trailing whitespace characters.
+ testOne("this is a tes|t ", "this is a |test ", "this is a |test ");
+ testOne("this is a test| ", "this is a |test ", "this is a test | ");
+ testOne("this is a test | ", "this is a |test ", "this is a test |");
+ testOne("this is a test |", "this is a test |", "this is a test |");
+ // Long line with one trailing whitespace character.
+ testOne("|testtest ", "testtest| ", "testtest |");
+ testOne("t|esttest ", "testtest| ", "testtest |");
+ testOne("testte|st ", "testtest| ", "testtest |");
+ testOne("testtes|t ", "testtest| ", "testtest |");
+ testOne("testtest| ", "testtest| ", "testtest |");
+ testOne("testtest |", "testtest |", "testtest |");
+ // Long line with two trailing whitespace characters.
+ testOne("|testtest ", "testtest| ", "testtest | ");
+ testOne("t|esttest ", "testtest| ", "testtest | ");
+ testOne("testte|st ", "testtest| ", "testtest | ");
+ testOne("testtes|t ", "testtest| ", "testtest | ");
+ testOne("testtest| ", "testtest| ", "testtest | ");
+ testOne("testtest | ", "testtest | ", "testtest |");
+ testOne("testtest |", "testtest |");
+ }
+
+ @Test
+ public void testAdjustBreakOffsetToWordLeadingSpace() {
+ // One leading space
+ testOne("| this is a test", " |this is a test");
+ testOne(" |this is a test", " |this is a test");
+ testOne(" t|his is a test", " |this is a test");
+ testOne(" th|is is a test", " |this is a test");
+ testOne(" thi|s is a test", " |this is a test");
+ testOne(" this| is a test", " |this is a test", " this |is a test");
+ testOne(" this |is a test", " this |is a test");
+ testOne(" this i|s a test", " this |is a test");
+ // Two leading spaces
+ testOne("| this is a test", " | this is a test");
+ testOne(" | this is a test", " | this is a test", " |this is a test");
+ testOne(" |this is a test", " |this is a test");
+ testOne(" t|his is a test", " |this is a test");
+ testOne(" th|is is a test", " |this is a test");
+ testOne(" thi|s is a test", " |this is a test");
+ testOne(" this| is a test", " |this is a test", " this |is a test");
+ testOne(" this |is a test", " this |is a test");
+ testOne(" this i|s a test", " this |is a test");
+ }
+
+ private static final class TextWithCaret {
+ public final String text;
+ public final int caret;
+
+ public TextWithCaret(String text, int caret) {
+ this(text, caret, true);
+ }
+
+ private TextWithCaret(String text, int caret, boolean check) {
+ if (text == null) {
+ throw new NullPointerException();
+ }
+ if (caret < 0 || caret > text.length()) {
+ throw new IllegalArgumentException();
+ }
+ this.text = text;
+ this.caret = caret;
+
+ if (check && !equals(fromEncoded(toString()))) {
+ throw new AssertionError();
+ }
+ }
+
+ /**
+ * @param encoded string containing a single '|' character indicating the caret position
+ */
+ public static TextWithCaret fromEncoded(String encoded) {
+ final StringBuilder psb = new StringBuilder();
+ Integer caret = null;
+ for (char ec : encoded.toCharArray()) {
+ if (ec == '|') {
+ if (caret != null) {
+ throw new IllegalArgumentException("Multiple caret positions specified");
+ }
+ caret = psb.length();
+ } else {
+ psb.append(ec);
+ }
+ }
+ if (caret == null) {
+ throw new IllegalArgumentException("No caret position specified");
+ }
+ return new TextWithCaret(psb.toString(), caret, false);
+ }
+
+ @Override
+ public String toString() {
+ return text.substring(0, caret) + '|' + text.substring(caret);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (!(obj instanceof TextWithCaret)) {
+ return false;
+ }
+ TextWithCaret other = (TextWithCaret) obj;
+ return this.text.equals(other.text)
+ && this.caret == other.caret;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(text, caret);
+ }
+ }
+}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists