Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a way to let the user provide a custom text wrapping function #386

Open
jbfro opened this issue Apr 30, 2024 · 7 comments
Open

Add a way to let the user provide a custom text wrapping function #386

jbfro opened this issue Apr 30, 2024 · 7 comments

Comments

@jbfro
Copy link

jbfro commented Apr 30, 2024

Even though the current text wrapping works fine, it is pretty basic ; not to say dumb .. It may break text in the middle of a word.

The solution to this situation is to let the user/developer specify a custom text wrapping function which better matches ones expectations.

As an alternative: I tried to rework the current text in place of the widget rendering. This was awful and didn't work pretty well when the text gets changed.

I made a patch which adds this feature to TextBox components.

@peterbrittain
Copy link
Owner

So if I understand correctly, you would really like the TextBox to support line wrapping at the end of the last full word before the end of the TextBox instead of the current logic. Is that right?

@jbfro
Copy link
Author

jbfro commented May 1, 2024

That is right (only when the line_wrap parameter is set to True). And let the user chose the way the text is wrapped (break on line width, break on word, add extra space before new subsequent lines, etc.)

The easiest way of doing I came up with is to add an extra parameter to the TextBox constructor, named wrap_line_cb for instance, which would default to an internal callback (i.e.: the current implementation of line wrap) if none is given.

That callback would take three parameters: line, limit, and unicode_aware as the current implementation requires. That callback would return an array of lines to be processed by the internal reflow function.

Would you be open to a patch or a new branch so you can consider the change?

@peterbrittain
Copy link
Owner

I'm always open to patches to asciimatics! In this case, the only question I have is whether this should be callback or not. If you want to enforce line-wrapping that better aligns with what people expect from web forms, I would happily take that instead of the current logic.

@jbfro
Copy link
Author

jbfro commented May 2, 2024

I can provide the following patch (based on master) witch only affects the TextBox widget. The patch

  • changes the default text wrapping logic so that is does not break words.
  • adds an option to specify a custom text wrapping callback (this case can be useful if the user requires to change the default wrapping logic for some specific needs).

NOTE: It seems I cannot create a new branch to your repository and Github prevents me from attaching a patch file ..

From fd4eef4d4c10fdccf00cf019c4b30d4583ffd91e Mon Sep 17 00:00:00 2001
From: Jean-Baptiste FROMENTEAU <jb.fromenteau@gmail.com>
Date: Tue, 30 Apr 2024 17:00:33 +0200
Subject: [PATCH] Add option to change default text wrapping function

Signed-off-by: Jean-Baptiste FROMENTEAU <jb.fromenteau@gmail.com>
---
 asciimatics/widgets/textbox.py   | 21 +++++---
 asciimatics/widgets/utilities.py | 84 ++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 8 deletions(-)

diff --git a/asciimatics/widgets/textbox.py b/asciimatics/widgets/textbox.py
index 78a9ca4..199bb51 100644
--- a/asciimatics/widgets/textbox.py
+++ b/asciimatics/widgets/textbox.py
@@ -5,7 +5,7 @@ from asciimatics.event import KeyboardEvent, MouseEvent
 from asciimatics.screen import Screen
 from asciimatics.strings import ColouredText
 from asciimatics.widgets.widget import Widget
-from asciimatics.widgets.utilities import _find_min_start, _enforce_width, _get_offset
+from asciimatics.widgets.utilities import _find_min_start, _enforce_width, _get_offset, _wrap_line
 
 # Logging
 logger = getLogger(__name__)
@@ -19,10 +19,10 @@ class TextBox(Widget):
     """
 
     __slots__ = ["_line", "_column", "_start_line", "_start_column", "_required_height",
-                 "_as_string", "_line_wrap", "_on_change", "_reflowed_text_cache", "_parser",
-                 "_hide_cursor", "_auto_scroll"]
+                 "_as_string", "_line_wrap", "_line_wrap_cb", "_on_change", "_reflowed_text_cache",
+                 "_parser", "_hide_cursor", "_auto_scroll"]
 
-    def __init__(self, height, label=None, name=None, as_string=False, line_wrap=False, parser=None,
+    def __init__(self, height, label=None, name=None, as_string=False, line_wrap=False, line_wrap_cb=None, parser=None,
                  on_change=None, readonly=False, **kwargs):
         """
         :param height: The required number of input lines for this TextBox.
@@ -31,6 +31,7 @@ class TextBox(Widget):
         :param as_string: Use string with newline separator instead of a list
             for the value of this widget.
         :param line_wrap: Whether to wrap at the end of the line.
+        :param line_wrap_cb: Optional callback to execute for wrapping a line.
         :param parser: Optional parser to colour text.
         :param on_change: Optional function to call when text changes.
         :param readonly: Whether the widget prevents user input to change values.  Default is False.
@@ -46,6 +47,9 @@ class TextBox(Widget):
         self._required_height = height
         self._as_string = as_string
         self._line_wrap = line_wrap
+        if (line_wrap_cb is None):
+            line_wrap_cb = self._default_line_wrap_cb
+        self._line_wrap_cb = line_wrap_cb
         self._parser = parser
         self._on_change = on_change
         self._reflowed_text_cache = None
@@ -258,6 +262,9 @@ class TextBox(Widget):
     def required_height(self, offset, width):
         return self._required_height
 
+    def _default_line_wrap_cb(self, line, limit, unicode_aware):
+        return _wrap_line(line, limit, unicode_aware)
+
     @property
     def _reflowed_text(self):
         """
@@ -272,13 +279,11 @@ class TextBox(Widget):
                 limit = self._w - self._offset
                 for i, line in enumerate(self._value):
                     column = 0
-                    while self.string_len(str(line)) >= limit:
-                        sub_string = _enforce_width(
-                            line, limit, self._frame.canvas.unicode_aware)
+                    wrapped = self._line_wrap_cb(line, max(1, limit-1), self._frame.canvas.unicode_aware)
+                    for sub_string in wrapped:
                         self._reflowed_text_cache.append((sub_string, i, column))
                         line = line[len(sub_string):]
                         column += len(sub_string)
-                    self._reflowed_text_cache.append((line, i, column))
             else:
                 self._reflowed_text_cache = [(x, i, 0) for i, x in enumerate(self._value)]
 
diff --git a/asciimatics/widgets/utilities.py b/asciimatics/widgets/utilities.py
index d2550dd..7b20c62 100644
--- a/asciimatics/widgets/utilities.py
+++ b/asciimatics/widgets/utilities.py
@@ -108,6 +108,90 @@ THEMES = {
 }
 
 
+def _str_len(text, unicode_aware=True):
+    """
+    Get the length of a text considering the unicode character length.
+
+    :param text: The text to be measured
+    :return: The text length.
+    """
+    size = 0
+    if unicode_aware:
+        for i, char in enumerate(str(text)):
+            c_width = wcwidth(char) if ord(char) >= 256 else 1
+            size += c_width
+    else:
+        size = len(text)
+    return size
+
+def _str_substring(text, width, unicode_aware=True):
+    """
+    Get a sub-string of specific length from text considering the unicode character length.
+
+    :param text: The text to sub-string.
+    :param width: The maximum resulting sub-string length.
+    :return: The sub-string text.
+    """
+    if unicode_aware:
+        size = 0
+        for i, char in enumerate(str(text)):
+            c_width = wcwidth(char) if ord(char) >= 256 else 1
+            if size + c_width > width:
+                return text[0:i]
+            size += c_width
+        return text
+    else:
+        return text[0:min(len(text), width)]
+
+def _wrap_line(text, width, unicode_aware=True):
+    """
+    Wrap a single line text to fit within the specified width.
+
+    :param line: The line to be wrapped
+    :param limit: The desired maximum line length
+    :param unicode_aware: If True, considers Unicode char length
+    :return: An array of string(s) representing the wrapped input line
+    """
+    BREAKABLE_CHARS = [" ", "\t"]
+
+    if _str_len(text, unicode_aware) < width:
+        return [text]
+
+    wrapped = []
+
+    text_length = len(text)
+
+    while text_length > 0:
+        line = _str_substring(text, width, unicode_aware)
+        line_length = _str_len(line, unicode_aware)
+
+        if line_length < width:
+            wrapped.append(line)
+            break
+
+        line_len = len(line)
+        text = text[line_len:]
+        text_length -= line_len
+
+        # ensure the line ends with a white-space (.i.e.: it does not break in
+        # the middle of a word)
+        extra_offset = line_length
+        while (extra_offset > 0) and (line[extra_offset-1] not in BREAKABLE_CHARS):
+            extra_offset -= 1
+
+        # if the resulting line is not empty, then it has a white-space and
+        # we can break it in a clean way
+        if extra_offset != 0:
+            extra = line[extra_offset:]
+            text = extra + "" + text
+            text_length += len(extra)
+
+            line = line[:extra_offset]
+
+        wrapped.append(line)
+
+    return wrapped
+
 def _enforce_width(text, width, unicode_aware=True):
     """
     Enforce a displayed piece of text to be a certain number of cells wide.  This takes into
-- 
2.25.1

@peterbrittain
Copy link
Owner

Thanks. Starting from that, I've tried to remove the callback and then minimize the changes to just add word breaks. How about this instead?

diff --git a/asciimatics/widgets/textbox.py b/asciimatics/widgets/textbox.py
index 78a9ca4..c2945f9 100644
--- a/asciimatics/widgets/textbox.py
+++ b/asciimatics/widgets/textbox.py
@@ -274,7 +274,7 @@ class TextBox(Widget):
                     column = 0
                     while self.string_len(str(line)) >= limit:
                         sub_string = _enforce_width(
-                            line, limit, self._frame.canvas.unicode_aware)
+                            line, limit, self._frame.canvas.unicode_aware, split_on_words=True)
                         self._reflowed_text_cache.append((sub_string, i, column))
                         line = line[len(sub_string):]
                         column += len(sub_string)
diff --git a/asciimatics/widgets/utilities.py b/asciimatics/widgets/utilities.py
index d2550dd..43e25ee 100644
--- a/asciimatics/widgets/utilities.py
+++ b/asciimatics/widgets/utilities.py
@@ -108,13 +108,15 @@ THEMES = {
 }


-def _enforce_width(text, width, unicode_aware=True):
+def _enforce_width(text, width, unicode_aware=True, split_on_words=False):
     """
     Enforce a displayed piece of text to be a certain number of cells wide.  This takes into
     account double-width characters used in CJK languages.

     :param text: The text to be truncated
     :param width: The screen cell width to enforce
+    :param unicode_aware: Whether the text needs unicode-aware handling.
+    :param split_on_words: Whether to respect word boundaries when splitting.
     :return: The resulting truncated text
     """
     # Double-width strings cannot be more than twice the string length, so no need to try
@@ -123,12 +125,15 @@ def _enforce_width(text, width, unicode_aware=True):
         return text

     # Can still optimize performance if we are not handling unicode characters.
-    if unicode_aware:
+    if unicode_aware or split_on_words:
         size = 0
+        last_space = 9999999999
         for i, char in enumerate(str(text)):
             c_width = wcwidth(char) if ord(char) >= 256 else 1
+            if split_on_words and char in (" ", "\t"):
+                last_space = i + 1
             if size + c_width > width:
-                return text[0:i]
+                return text[0:min(i, last_space)]
             size += c_width
     elif len(text) + 1 > width:
         return text[0:width]

@jbfro
Copy link
Author

jbfro commented May 3, 2024

The callback part was a nice to have for very specific cases. I just tested your patch, it looks okay to me. Thanks for considering my request! \o/

@peterbrittain
Copy link
Owner

Some "very specific cases" sounds like it's possibly a different widget to me. We could look into those and see if it makes sense to add them to the standard set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants