From b2aba6db352027c6f7d5db0256e3483eb5a6aded Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 21 Jan 2020 15:32:32 +0000 Subject: [PATCH 1/4] Initial attempt to make toggleInlineFormat paragraph-aware --- src/editor/deserialize.js | 2 +- src/editor/operations.js | 70 +++++++++++---- test/editor/mock.js | 10 +++ test/editor/operations-test.js | 154 +++++++++++++++++++++++++++++++++ test/editor/range-test.js | 12 +-- 5 files changed, 221 insertions(+), 27 deletions(-) create mode 100644 test/editor/operations-test.js diff --git a/src/editor/deserialize.js b/src/editor/deserialize.js index 1fdbf9490c..7ba4c3eda3 100644 --- a/src/editor/deserialize.js +++ b/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 const parts = lines.reduce((parts, line, i) => { if (isQuotedMessage) { parts.push(partCreator.plain(QUOTE_LINE_PREFIX)); diff --git a/src/editor/operations.js b/src/editor/operations.js index e2661faf59..6bae60e6b8 100644 --- a/src/editor/operations.js +++ b/src/editor/operations.js @@ -104,23 +104,63 @@ 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; + // let seenNewlines = 0; + 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(), 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); + const isBlank = part => !part.text || !/\S/.test(part.text); + const isNL = part => part.type === "newline"; - 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)); + // 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; + } + + 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; + } } + if (startIndex < parts.length) { + // TODO don't use parts.length here to clean up any trailing cruft + paragraphIndexes.push([startIndex, parts.length]); + } + + // 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); } diff --git a/test/editor/mock.js b/test/editor/mock.js index bb1a51d14b..6de65cf23d 100644 --- a/test/editor/mock.js +++ b/test/editor/mock.js @@ -67,3 +67,13 @@ export function createPartCreator(completions = []) { }; return new PartCreator(new MockRoom(), new MockClient(), autoCompleteCreator); } + +export function createRenderer() { + const render = (c) => { + render.caret = c; + render.count += 1; + }; + render.count = 0; + render.caret = null; + return render; +} diff --git a/test/editor/operations-test.js b/test/editor/operations-test.js new file mode 100644 index 0000000000..591166759c --- /dev/null +++ b/test/editor/operations-test.js @@ -0,0 +1,154 @@ +/* +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 {getLineAndNodePosition} from "../../src/editor/caret"; +import EditorModel from "../../src/editor/model"; +import {createPartCreator, createRenderer} from "./mock"; +import {toggleInlineFormat} from "../../src/editor/operations"; + +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"}, + {"text": "\n", "type": "newline"}, + {"text": "how are you doing?", "type": "plain"}, + ]); + toggleInlineFormat(range, "**"); + expect(model.serializeParts()).toEqual([ + {"text": "hello **world,", "type": "plain"}, + {"text": "\n", "type": "newline"}, + {"text": "how** are you doing?", "type": "plain"}, + ]); + }); + + 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"}, + {"text": "\n", "type": "newline"}, + {"text": "how are you doing?", "type": "plain"}, + {"text": "\n", "type": "newline"}, + {"text": "\n", "type": "newline"}, + {"text": "new paragraph", "type": "plain"}, + ]); + toggleInlineFormat(range, "__"); + expect(model.serializeParts()).toEqual([ + {"text": "__hello world,", "type": "plain"}, + {"text": "\n", "type": "newline"}, + {"text": "how are you doing?__", "type": "plain"}, + {"text": "\n", "type": "newline"}, + {"text": "\n", "type": "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"}, + {"text": "\n", "type": "newline"}, + {"text": "how are you doing?", "type": "plain"}, + {"text": "\n", "type": "newline"}, + {"text": "\n", "type": "newline"}, + {"text": "new paragraph", "type": "plain"}, + ]); + }); + }); +}); diff --git a/test/editor/range-test.js b/test/editor/range-test.js index 53fb6cb765..b69ed9eb53 100644 --- a/test/editor/range-test.js +++ b/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"; From 9a530a72f6d27dd67d338533fe3823c880d7ff77 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 21 Jan 2020 15:36:25 +0000 Subject: [PATCH 2/4] delint --- test/editor/operations-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/editor/operations-test.js b/test/editor/operations-test.js index 591166759c..872cc78bdb 100644 --- a/test/editor/operations-test.js +++ b/test/editor/operations-test.js @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {getLineAndNodePosition} from "../../src/editor/caret"; import EditorModel from "../../src/editor/model"; import {createPartCreator, createRenderer} from "./mock"; import {toggleInlineFormat} from "../../src/editor/operations"; From 832da062cccc814fe5748e60a9c71f03290a6eff Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 22 Jan 2020 13:37:27 +0000 Subject: [PATCH 3/4] Improve trailing spurious breaks + tests --- src/editor/deserialize.js | 2 +- src/editor/operations.js | 16 +++++--- test/editor/operations-test.js | 67 ++++++++++++++++++++++++++-------- 3 files changed, 63 insertions(+), 22 deletions(-) diff --git a/src/editor/deserialize.js b/src/editor/deserialize.js index 7ba4c3eda3..190963f357 100644 --- a/src/editor/deserialize.js +++ b/src/editor/deserialize.js @@ -250,7 +250,7 @@ function parseHtmlMessage(html, partCreator, isQuotedMessage) { } export function parsePlainTextMessage(body, partCreator, isQuotedMessage) { - const lines = body.split(/\r\n|\r|\n/g); // split on any new-line combination not just \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)); diff --git a/src/editor/operations.js b/src/editor/operations.js index 6bae60e6b8..d0115d9ca7 100644 --- a/src/editor/operations.js +++ b/src/editor/operations.js @@ -100,6 +100,10 @@ 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; @@ -113,14 +117,12 @@ export function toggleInlineFormat(range, prefix, suffix = prefix) { // - 2 newline parts in sequence // - newline part, plain(), newline part - const isBlank = part => !part.text || !/\S/.test(part.text); - const isNL = part => part.type === "newline"; - // 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; } + // 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; @@ -129,9 +131,11 @@ export function toggleInlineFormat(range, prefix, suffix = prefix) { startIndex = i + 1; } } - if (startIndex < parts.length) { - // TODO don't use parts.length here to clean up any trailing cruft - paragraphIndexes.push([startIndex, parts.length]); + + 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 diff --git a/test/editor/operations-test.js b/test/editor/operations-test.js index 872cc78bdb..90a9812306 100644 --- a/test/editor/operations-test.js +++ b/test/editor/operations-test.js @@ -18,6 +18,8 @@ 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', () => { @@ -93,17 +95,54 @@ describe('editor/operations: formatting operations', () => { expect(range.parts.map(p => p.text).join("")).toBe("world,\nhow"); expect(model.serializeParts()).toEqual([ {"text": "hello world,", "type": "plain"}, - {"text": "\n", "type": "newline"}, + SERIALIZED_NEWLINE, {"text": "how are you doing?", "type": "plain"}, ]); toggleInlineFormat(range, "**"); expect(model.serializeParts()).toEqual([ {"text": "hello **world,", "type": "plain"}, - {"text": "\n", "type": "newline"}, + 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(); @@ -116,36 +155,34 @@ describe('editor/operations: formatting operations', () => { pc.plain("new paragraph"), ], pc, renderer); - let range = model.startRange(model.positionForOffset(0, true), - model.getPositionAtEnd()); // select-all + let range = model.startRange(model.positionForOffset(0, true), model.getPositionAtEnd()); // select-all expect(model.serializeParts()).toEqual([ {"text": "hello world,", "type": "plain"}, - {"text": "\n", "type": "newline"}, + SERIALIZED_NEWLINE, {"text": "how are you doing?", "type": "plain"}, - {"text": "\n", "type": "newline"}, - {"text": "\n", "type": "newline"}, + SERIALIZED_NEWLINE, + SERIALIZED_NEWLINE, {"text": "new paragraph", "type": "plain"}, ]); toggleInlineFormat(range, "__"); expect(model.serializeParts()).toEqual([ {"text": "__hello world,", "type": "plain"}, - {"text": "\n", "type": "newline"}, + SERIALIZED_NEWLINE, {"text": "how are you doing?__", "type": "plain"}, - {"text": "\n", "type": "newline"}, - {"text": "\n", "type": "newline"}, + SERIALIZED_NEWLINE, + SERIALIZED_NEWLINE, {"text": "__new paragraph__", "type": "plain"}, ]); - range = model.startRange(model.positionForOffset(0, true), - model.getPositionAtEnd()); // select-all + 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"}, - {"text": "\n", "type": "newline"}, + SERIALIZED_NEWLINE, {"text": "how are you doing?", "type": "plain"}, - {"text": "\n", "type": "newline"}, - {"text": "\n", "type": "newline"}, + SERIALIZED_NEWLINE, + SERIALIZED_NEWLINE, {"text": "new paragraph", "type": "plain"}, ]); }); From 13bb719a890818e15ea14959fb5fdcab55cf54bc Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 23 Jan 2020 11:22:22 +0000 Subject: [PATCH 4/4] Add comment for operations loop --- src/editor/operations.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/operations.js b/src/editor/operations.js index d0115d9ca7..d677d7016c 100644 --- a/src/editor/operations.js +++ b/src/editor/operations.js @@ -111,7 +111,7 @@ export function toggleInlineFormat(range, prefix, suffix = prefix) { // compute paragraph [start, end] indexes const paragraphIndexes = []; let startIndex = 0; - // let seenNewlines = 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