Merge pull request #3891 from matrix-org/t3chguy/fix_multi_paragraph_formatting

Fix paragraph-awareness of the composer formatting features

src/editor/deserialize.js

@@ -250,7 +250,7 @@ function parseHtmlMessage(html, partCreator, isQuotedMessage) {
 }
 
 export function parsePlainTextMessage(body, partCreator, isQuotedMessage) {
-    const lines = body.split("\n");
+    const lines = body.split(/\r\n|\r|\n/g); // split on any new-line combination not just \n, collapses \r\n
     const parts = lines.reduce((parts, line, i) => {
         if (isQuotedMessage) {
             parts.push(partCreator.plain(QUOTE_LINE_PREFIX));

src/editor/operations.js

@@ -100,27 +100,71 @@ export function formatRangeAsCode(range) {
     replaceRangeAndExpandSelection(range, parts);
 }
 
+// parts helper methods
+const isBlank = part => !part.text || !/\S/.test(part.text);
+const isNL = part => part.type === "newline";
+
 export function toggleInlineFormat(range, prefix, suffix = prefix) {
     const {model, parts} = range;
     const {partCreator} = model;
 
-    const isFormatted = parts.length &&
-        parts[0].text.startsWith(prefix) &&
-        parts[parts.length - 1].text.endsWith(suffix);
+    // compute paragraph [start, end] indexes
+    const paragraphIndexes = [];
+    let startIndex = 0;
+    // start at i=2 because we look at i and up to two parts behind to detect paragraph breaks at their end
+    for (let i = 2; i < parts.length; i++) {
+        // paragraph breaks can be denoted in a multitude of ways,
+        // - 2 newline parts in sequence
+        // - newline part, plain(<empty or just spaces>), newline part
 
-    if (isFormatted) {
-        // remove prefix and suffix
-        const partWithoutPrefix = parts[0].serialize();
-        partWithoutPrefix.text = partWithoutPrefix.text.substr(prefix.length);
-        parts[0] = partCreator.deserializePart(partWithoutPrefix);
+        // bump startIndex onto the first non-blank after the paragraph ending
+        if (isBlank(parts[i - 2]) && isNL(parts[i - 1]) && !isNL(parts[i]) && !isBlank(parts[i])) {
+            startIndex = i;
+        }
 
-        const partWithoutSuffix = parts[parts.length - 1].serialize();
-        const suffixPartText = partWithoutSuffix.text;
-        partWithoutSuffix.text = suffixPartText.substring(0, suffixPartText.length - suffix.length);
-        parts[parts.length - 1] = partCreator.deserializePart(partWithoutSuffix);
-    } else {
-        parts.unshift(partCreator.plain(prefix));
-        parts.push(partCreator.plain(suffix));
+        // if at a paragraph break, store the indexes of the paragraph
+        if (isNL(parts[i - 1]) && isNL(parts[i])) {
+            paragraphIndexes.push([startIndex, i - 1]);
+            startIndex = i + 1;
+        } else if (isNL(parts[i - 2]) && isBlank(parts[i - 1]) && isNL(parts[i])) {
+            paragraphIndexes.push([startIndex, i - 2]);
+            startIndex = i + 1;
+        }
     }
+
+    const lastNonEmptyPart = parts.map(isBlank).lastIndexOf(false);
+    // If we have not yet included the final paragraph then add it now
+    if (startIndex <= lastNonEmptyPart) {
+        paragraphIndexes.push([startIndex, lastNonEmptyPart + 1]);
+    }
+
+    // keep track of how many things we have inserted as an offset:=0
+    let offset = 0;
+    paragraphIndexes.forEach(([startIndex, endIndex]) => {
+        // for each paragraph apply the same rule
+        const base = startIndex + offset;
+        const index = endIndex + offset;
+
+        const isFormatted = (index - base > 0) &&
+            parts[base].text.startsWith(prefix) &&
+            parts[index - 1].text.endsWith(suffix);
+
+        if (isFormatted) {
+            // remove prefix and suffix
+            const partWithoutPrefix = parts[base].serialize();
+            partWithoutPrefix.text = partWithoutPrefix.text.substr(prefix.length);
+            parts[base] = partCreator.deserializePart(partWithoutPrefix);
+
+            const partWithoutSuffix = parts[index - 1].serialize();
+            const suffixPartText = partWithoutSuffix.text;
+            partWithoutSuffix.text = suffixPartText.substring(0, suffixPartText.length - suffix.length);
+            parts[index - 1] = partCreator.deserializePart(partWithoutSuffix);
+        } else {
+            parts.splice(index, 0, partCreator.plain(suffix)); // splice in the later one first to not change offset
+            parts.splice(base, 0, partCreator.plain(prefix));
+            offset += 2; // offset index to account for the two items we just spliced in
+        }
+    });
+
     replaceRangeAndExpandSelection(range, parts);
 }

test/editor/operations-test.js

@@ -0,0 +1,190 @@
+/*
+Copyright 2020 The Matrix.org Foundation C.I.C.
+
+Licensed 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.
+*/
+
+import EditorModel from "../../src/editor/model";
+import {createPartCreator, createRenderer} from "./mock";
+import {toggleInlineFormat} from "../../src/editor/operations";
+
+const SERIALIZED_NEWLINE = {"text": "\n", "type": "newline"};
+
+describe('editor/operations: formatting operations', () => {
+    describe('toggleInlineFormat', () => {
+        it('works for words', () => {
+            const renderer = createRenderer();
+            const pc = createPartCreator();
+            const model = new EditorModel([
+                pc.plain("hello world!"),
+            ], pc, renderer);
+
+            const range = model.startRange(model.positionForOffset(6, false),
+                model.positionForOffset(11, false));  // around "world"
+
+            expect(range.parts[0].text).toBe("world");
+            expect(model.serializeParts()).toEqual([{"text": "hello world!", "type": "plain"}]);
+            toggleInlineFormat(range, "_");
+            expect(model.serializeParts()).toEqual([{"text": "hello _world_!", "type": "plain"}]);
+        });
+
+        it('works for parts of words', () => {
+            const renderer = createRenderer();
+            const pc = createPartCreator();
+            const model = new EditorModel([
+                pc.plain("hello world!"),
+            ], pc, renderer);
+
+            const range = model.startRange(model.positionForOffset(7, false),
+                model.positionForOffset(10, false));  // around "orl"
+
+            expect(range.parts[0].text).toBe("orl");
+            expect(model.serializeParts()).toEqual([{"text": "hello world!", "type": "plain"}]);
+            toggleInlineFormat(range, "*");
+            expect(model.serializeParts()).toEqual([{"text": "hello w*orl*d!", "type": "plain"}]);
+        });
+
+        it('works for around pills', () => {
+            const renderer = createRenderer();
+            const pc = createPartCreator();
+            const model = new EditorModel([
+                pc.plain("hello there "),
+                pc.atRoomPill("@room"),
+                pc.plain(", how are you doing?"),
+            ], pc, renderer);
+
+            const range = model.startRange(model.positionForOffset(6, false),
+                model.positionForOffset(30, false));  // around "there @room, how are you"
+
+            expect(range.parts.map(p => p.text).join("")).toBe("there @room, how are you");
+            expect(model.serializeParts()).toEqual([
+                {"text": "hello there ", "type": "plain"},
+                {"text": "@room", "type": "at-room-pill"},
+                {"text": ", how are you doing?", "type": "plain"},
+            ]);
+            toggleInlineFormat(range, "_");
+            expect(model.serializeParts()).toEqual([
+                {"text": "hello _there ", "type": "plain"},
+                {"text": "@room", "type": "at-room-pill"},
+                {"text": ", how are you_ doing?", "type": "plain"},
+            ]);
+        });
+
+        it('works for a paragraph', () => {
+            const renderer = createRenderer();
+            const pc = createPartCreator();
+            const model = new EditorModel([
+                pc.plain("hello world,"),
+                pc.newline(),
+                pc.plain("how are you doing?"),
+            ], pc, renderer);
+
+            const range = model.startRange(model.positionForOffset(6, false),
+                model.positionForOffset(16, false));  // around "world,\nhow"
+
+            expect(range.parts.map(p => p.text).join("")).toBe("world,\nhow");
+            expect(model.serializeParts()).toEqual([
+                {"text": "hello world,", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                {"text": "how are you doing?", "type": "plain"},
+            ]);
+            toggleInlineFormat(range, "**");
+            expect(model.serializeParts()).toEqual([
+                {"text": "hello **world,", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                {"text": "how** are you doing?", "type": "plain"},
+            ]);
+        });
+
+        it('works for a paragraph with spurious breaks around it in selected range', () => {
+            const renderer = createRenderer();
+            const pc = createPartCreator();
+            const model = new EditorModel([
+                pc.newline(),
+                pc.newline(),
+                pc.plain("hello world,"),
+                pc.newline(),
+                pc.plain("how are you doing?"),
+                pc.newline(),
+                pc.newline(),
+            ], pc, renderer);
+
+            const range = model.startRange(model.positionForOffset(0, false), model.getPositionAtEnd());  // select-all
+
+            expect(range.parts.map(p => p.text).join("")).toBe("\n\nhello world,\nhow are you doing?\n\n");
+            expect(model.serializeParts()).toEqual([
+                SERIALIZED_NEWLINE,
+                SERIALIZED_NEWLINE,
+                {"text": "hello world,", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                {"text": "how are you doing?", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                SERIALIZED_NEWLINE,
+            ]);
+            toggleInlineFormat(range, "**");
+            expect(model.serializeParts()).toEqual([
+                SERIALIZED_NEWLINE,
+                SERIALIZED_NEWLINE,
+                {"text": "**hello world,", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                {"text": "how are you doing?**", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                SERIALIZED_NEWLINE,
+            ]);
+        });
+
+        it('works for multiple paragraph', () => {
+            const renderer = createRenderer();
+            const pc = createPartCreator();
+            const model = new EditorModel([
+                pc.plain("hello world,"),
+                pc.newline(),
+                pc.plain("how are you doing?"),
+                pc.newline(),
+                pc.newline(),
+                pc.plain("new paragraph"),
+            ], pc, renderer);
+
+            let range = model.startRange(model.positionForOffset(0, true), model.getPositionAtEnd()); // select-all
+
+            expect(model.serializeParts()).toEqual([
+                {"text": "hello world,", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                {"text": "how are you doing?", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                SERIALIZED_NEWLINE,
+                {"text": "new paragraph", "type": "plain"},
+            ]);
+            toggleInlineFormat(range, "__");
+            expect(model.serializeParts()).toEqual([
+                {"text": "__hello world,", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                {"text": "how are you doing?__", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                SERIALIZED_NEWLINE,
+                {"text": "__new paragraph__", "type": "plain"},
+            ]);
+            range = model.startRange(model.positionForOffset(0, true), model.getPositionAtEnd()); // select-all
+            console.log("RANGE", range.parts);
+            toggleInlineFormat(range, "__");
+            expect(model.serializeParts()).toEqual([
+                {"text": "hello world,", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                {"text": "how are you doing?", "type": "plain"},
+                SERIALIZED_NEWLINE,
+                SERIALIZED_NEWLINE,
+                {"text": "new paragraph", "type": "plain"},
+            ]);
+        });
+    });
+});

test/editor/range-test.js

@@ -15,17 +15,7 @@ limitations under the License.
 */
 
 import EditorModel from "../../src/editor/model";
-import {createPartCreator} from "./mock";
-
-function createRenderer() {
-    const render = (c) => {
-        render.caret = c;
-        render.count += 1;
-    };
-    render.count = 0;
-    render.caret = null;
-    return render;
-}
+import {createPartCreator, createRenderer} from "./mock";
 
 const pillChannel = "#riot-dev:matrix.org";