Merge pull request #75 from AtkinsSJ/wrapping-whitespace

Improve text wrapping

src/puter-shell/coreutils/coreutil_lib/help.js

@@ -41,6 +41,9 @@ export const printUsage = async (command, out, vars) => {
     const colorOptionArgument = text => {
         return `\x1B[91m${text}\x1B[0m`;
     };
+    const wrap = text => {
+        return wrapText(text, vars.size.cols).join('\n') + '\n';
+    }
 
     await heading('Usage');
     if (!usage) {
@@ -62,10 +65,7 @@ export const printUsage = async (command, out, vars) => {
     }
 
     if (description) {
-        const wrappedLines = wrapText(description, vars.size.cols);
-        for (const line of wrappedLines) {
-            await out.write(`${line}\n`);
-        }
+        await out.write(wrap(description));
         await out.write(`\n`);
     }
 
@@ -127,8 +127,7 @@ export const printUsage = async (command, out, vars) => {
     if (helpSections) {
         for (const [title, contents] of Object.entries(helpSections)) {
             await heading(title);
-            // FIXME: Wrap the text nicely.
-            await out.write(contents);
+            await out.write(wrap(contents));
             await out.write('\n\n');
         }
     }

src/util/wrap-text.js

@@ -16,41 +16,96 @@
  * You should have received a copy of the GNU Affero General Public License
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
-// TODO: Detect ANSI escape sequences in the text and treat them as 0 width?
+export function lengthIgnoringEscapes(text) {
+    const escape = '\x1b';
+    // There are a lot of different ones, but we only use graphics-mode ones, so only parse those for now.
+    // TODO: Parse other escape sequences as needed.
+    // Format is: ESC, '[', DIGIT, 0 or more characters, and then 'm'
+    const escapeSequenceRegex = /^\x1B\[\d.*?m/;
+
+    let length = 0;
+    for (let i = 0; i < text.length; i++) {
+        const char = text[i];
+        if (char === escape) {
+            // Consume an ANSI escape sequence
+            const match = text.substring(i).match(escapeSequenceRegex);
+            if (match) {
+                i += match[0].length - 1;
+            }
+            continue;
+        }
+        length++;
+    }
+    return length;
+}
+
 // TODO: Ensure this works with multi-byte characters (UTF-8)
 export const wrapText = (text, width) => {
+    const whitespaceChars = ' \t'.split('');
+    const isWhitespace = c => {
+        return whitespaceChars.includes(c);
+    };
+
     // If width was invalid, just return the original text as a failsafe.
     if (typeof width !== 'number' || width < 1)
         return [text];
 
     const lines = [];
-    // This reduces all whitespace to single space characters. Is that a problem?
-    const words = text.split(/\s+/);
-
     let currentLine = '';
     const splitWordIfTooLong = (word) => {
-        while (word.length > width) {
+        while (lengthIgnoringEscapes(word) > width) {
             lines.push(word.substring(0, width - 1) + '-');
             word = word.substring(width - 1);
         }
 
         currentLine = word;
     };
 
-    for (let word of words) {
-        if (currentLine.length === 0) {
-            splitWordIfTooLong(word);
+    for (let i = 0; i < text.length; i++) {
+        const char = text.charAt(i);
+        // Handle special characters
+        if (char === '\n') {
+            lines.push(currentLine.trimEnd());
+            currentLine = '';
+            // Don't skip whitespace after a newline, to allow for indentation.
+            continue;
+        }
+        // TODO: Handle \t?
+        if (/\S/.test(char)) {
+            // Grab next word
+            let word = char;
+            while ((i+1) < text.length && /\S/.test(text[i + 1])) {
+                word += text[i+1];
+                i++;
+            }
+            if (lengthIgnoringEscapes(currentLine) === 0) {
+                splitWordIfTooLong(word);
+                continue;
+            }
+            if ((lengthIgnoringEscapes(currentLine) + lengthIgnoringEscapes(word)) > width) {
+                // Next line
+                lines.push(currentLine.trimEnd());
+                splitWordIfTooLong(word);
+                continue;
+            }
+            currentLine += word;
             continue;
         }
-        if ((currentLine.length + 1 + word.length) > width) {
-            // Next line
-            lines.push(currentLine);
-            splitWordIfTooLong(word);
+
+        currentLine += char;
+        if (lengthIgnoringEscapes(currentLine) >= width) {
+            lines.push(currentLine.trimEnd());
+            currentLine = '';
+            // Skip whitespace at end of line.
+            while (isWhitespace(text[i + 1])) {
+                i++;
+            }
             continue;
         }
-        currentLine += ' ' + word;
     }
-    lines.push(currentLine);
+    if (currentLine.length >= 0) { // Not lengthIgnoringEscapes!
+        lines.push(currentLine);
+    }
 
     return lines;
 };

test/wrap-text.js

@@ -17,7 +17,7 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 import assert from 'assert';
-import { wrapText } from '../src/util/wrap-text.js';
+import { lengthIgnoringEscapes, wrapText } from '../src/util/wrap-text.js';
 
 describe('wrapText', () => {
     const testCases = [
@@ -51,19 +51,34 @@ describe('wrapText', () => {
             width: 0,
             output: ['Well, hello friends!'],
         },
+        {
+            description: 'should maintain existing newlines',
+            input: 'Well\nhello\n\nfriends!',
+            width: 20,
+            output: ['Well', 'hello', '', 'friends!'],
+        },
+        {
+            description: 'should maintain indentation after newlines',
+            input: 'Well\n      hello\n\nfriends!',
+            width: 20,
+            output: ['Well', '      hello', '', 'friends!'],
+        },
+        {
+            description: 'should ignore ansi escape sequences',
+            input: '\x1B[34;1mWell this is some text with ansi escape sequences\x1B[0m',
+            width: 20,
+            output: ['\x1B[34;1mWell this is some', 'text with ansi', 'escape sequences\x1B[0m'],
+        },
     ];
     for (const { description, input, width, output } of testCases) {
         it (description, () => {
             const result = wrapText(input, width);
             for (const line of result) {
                 if (typeof width === 'number' && width > 0) {
-                    assert.ok(line.length <= width, `Line is too long: '${line}`);
+                    assert.ok(lengthIgnoringEscapes(line) <= width, `Line is too long: '${line}'`);
                 }
             }
-            assert.equal(result.length, output.length, 'Wrong number of lines');
-            for (const i in result) {
-                assert.equal(result[i], output[i], `Line ${i} doesn't match: expected '${output[i]}', got '${result[i]}'`);
-            }
+            assert.equal('|' + result.join('|\n|') + '|', '|' + output.join('|\n|') + '|');
         });
     }
 })