[GitHub] sdedic commented on a change in pull request #598: [NETBEANS-977] Improve text layout in word wrap mode.

[GitBox <gi...@apache.org>]

sdedic commented on a change in pull request #598: [NETBEANS-977] Improve text layout in word wrap mode.
URL: https://github.com/apache/incubator-netbeans/pull/598#discussion_r216688614
 
 

 ##########
 File path: ide/editor.lib2/src/org/netbeans/modules/editor/lib2/view/HighlightsViewUtils.java
 ##########
 @@ -777,4 +756,85 @@ 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)
+                ret = preferredMaximumBreakOffset;
 
 Review comment:
   Use {} around if-ed statement; maybe return `preferredMaximumBreakOffset` immediately for better clarity (case `preferredMaximumBreakOffset == 0` handled at 801).
   
   If `bi.preceding(...) == 0`, that is boundary at char 0, is it OK to search forwards (as seen below) ?

----------------------------------------------------------------
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