Unverified Commit 6c12e399 authored by Renzo Olivares's avatar Renzo Olivares Committed by GitHub

Introduce ParagraphBoundary subclass for text editing (#116549)

* attempt to extend to paragraph

* second attempt

* clean up implementation

* clean up

* updates

* updates

* Fix implementation

* remove old

* update docs

* update docs

* fix analyzer

* Fix bug where new line character was selected and backwards selection failed

* remove print

* Add test for paragraph boundary

* Add text editing test for extending selection to paragraph for mac and ios

* rename to ExtendSelectionToParagraphBoundaryIntent

* fix analyzer

* Should default to downstream when collapsing selection

* get rid of _getParagraphAtOffset and move into getTextBoundaryAt

* Search for all line terminators

* iterate through code units instead of characters

* Address some reviewer comments"

* Add separate implementations for leading and trailing paragraph boundary methods

* Do not break after a carriage return if it is followed by a line feed

* test carriage return followed by a line feed

* more tests

* Do not continue if the line terminator is at the target text offset

* add hack to extend highlight to line terminator

* Revert "add hack to extend highlight to line terminator"

This reverts commit b4d3c434539b66c3c81c215e87c645b425902825.

* Revert "Do not continue if the line terminator is at the target text offset"

This reverts commit 789e1b838e54e7c25600bfa8852e59431ccaf5dc.

* Update ParagraphBoundary with latest TextBoundary changes

* Update implementation to iterate through indexes

* update getTrailingTextBoundaryAt to include the line terminator

* Updates

* more updates

* more updates

* updates

* updates

* Lets try this again

* clean up

* updates

* more updates

* updates

* fix

* Re-implement using custom paragraph boundary applying method

* Revert "Re-implement using custom paragraph boundary applying method"

This reverts commit cd2f7f4b6eb6726b28f82a43708812e06a49df95.

* Revert "fix"

This reverts commit 8ec1f8f58935cfb3eb86dc6afd2894537af4cf7b.

* updates

* Revert "updates"

This reverts commit 9dcca4a0031fe18ada9d6ffbbe77ba09918e82ae.

* Revert "Revert "fix""

This reverts commit 9cc1332cd3041badc472d0d223a106203e46afb8.

* Revert "Revert "Re-implement using custom paragraph boundary applying method""

This reverts commit 1acb606fb743fd840da20cca26d9a7c26accb71d.

* Fix paragraph boundaries

* Add failing test

* Address some comments

* group tests and fix analyzer

* fix typo

* fix remaining test

* updates

* more fixes and logs

* clean up and add another test

* Fix last test

* Add new test

* Clean up

* more clean up

* clean up comments

* address comments

* updates

* return null when position is out of bounds and 0 or end of text if appropriate

* Clean up cases

* Do not return null when OOB in the direction of iteration

* clean up

* simplify implementation thanks to LongCatIsLooong feedback

* Address comments

* Add line and paragraph separator

* Use _moveBeyondTextBoundary instead of custom _moveToParagraphBoundary

* Change some intent names and revert fromPosition change

* clean up docs

---------
Co-authored-by: 's avatarRenzo Olivares <roliv@google.com>
parent 7477d7ac
......@@ -123,9 +123,87 @@ class LineBoundary extends TextBoundary {
TextRange getTextBoundaryAt(int position) => _textLayout.getLineAtOffset(TextPosition(offset: max(position, 0)));
}
/// A text boundary that uses paragraphs as logical boundaries.
///
/// A paragraph is defined as the range between line terminators. If no
/// line terminators exist then the paragraph boundary is the entire document.
class ParagraphBoundary extends TextBoundary {
/// Creates a [ParagraphBoundary] with the text.
const ParagraphBoundary(this._text);
final String _text;
/// Returns the [int] representing the start position of the paragraph that
/// bounds the given `position`. The returned [int] is the position of the code unit
/// that follows the line terminator that encloses the desired paragraph.
@override
int? getLeadingTextBoundaryAt(int position) {
if (position < 0 || _text.isEmpty) {
return null;
}
if (position >= _text.length) {
return _text.length;
}
if (position == 0) {
return 0;
}
final List<int> codeUnits = _text.codeUnits;
int index = position;
if (index > 1 && codeUnits[index] == 0xA && codeUnits[index - 1] == 0xD) {
index -= 2;
} else if (TextLayoutMetrics.isLineTerminator(codeUnits[index])) {
index -= 1;
}
while (index > 0) {
if (TextLayoutMetrics.isLineTerminator(codeUnits[index])) {
return index + 1;
}
index -= 1;
}
return max(index, 0);
}
/// Returns the [int] representing the end position of the paragraph that
/// bounds the given `position`. The returned [int] is the position of the
/// code unit representing the trailing line terminator that encloses the
/// desired paragraph.
@override
int? getTrailingTextBoundaryAt(int position) {
if (position >= _text.length || _text.isEmpty) {
return null;
}
if (position < 0) {
return 0;
}
final List<int> codeUnits = _text.codeUnits;
int index = position;
while (!TextLayoutMetrics.isLineTerminator(codeUnits[index])) {
index += 1;
if (index == codeUnits.length) {
return index;
}
}
return index < codeUnits.length - 1
&& codeUnits[index] == 0xD
&& codeUnits[index + 1] == 0xA
? index + 2
: index + 1;
}
}
/// A text boundary that uses the entire document as logical boundary.
class DocumentBoundary extends TextBoundary {
/// Creates a [DocumentBoundary] with the text
/// Creates a [DocumentBoundary] with the text.
const DocumentBoundary(this._text);
final String _text;
......
......@@ -54,6 +54,25 @@ abstract class TextLayoutMetrics {
return true;
}
/// Check if the given code unit is a line terminator character.
///
/// Includes newline characters from ASCII
/// (https://www.unicode.org/standard/reports/tr13/tr13-5.html).
static bool isLineTerminator(int codeUnit) {
switch (codeUnit) {
case 0xA: // line feed
case 0xB: // vertical feed
case 0xC: // form feed
case 0xD: // carriage return
case 0x85: // new line
case 0x2028: // line separator
case 0x2029: // paragraph separator
return true;
default:
return false;
}
}
/// {@template flutter.services.TextLayoutMetrics.getLineAtOffset}
/// Return a [TextSelection] containing the line of the given [TextPosition].
/// {@endtemplate}
......
......@@ -303,8 +303,8 @@ class DefaultTextEditingShortcuts extends StatelessWidget {
const SingleActivator(LogicalKeyboardKey.arrowLeft, shift: true, alt: true): const ExtendSelectionToNextWordBoundaryOrCaretLocationIntent(forward: false),
const SingleActivator(LogicalKeyboardKey.arrowRight, shift: true, alt: true): const ExtendSelectionToNextWordBoundaryOrCaretLocationIntent(forward: true),
const SingleActivator(LogicalKeyboardKey.arrowUp, shift: true, alt: true): const ExtendSelectionVerticallyToAdjacentLineIntent(forward: false, collapseSelection: false),
const SingleActivator(LogicalKeyboardKey.arrowDown, shift: true, alt: true): const ExtendSelectionVerticallyToAdjacentLineIntent(forward: true, collapseSelection: false),
const SingleActivator(LogicalKeyboardKey.arrowUp, shift: true, alt: true): const ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent(forward: false),
const SingleActivator(LogicalKeyboardKey.arrowDown, shift: true, alt: true): const ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent(forward: true),
const SingleActivator(LogicalKeyboardKey.arrowLeft, meta: true): const ExtendSelectionToLineBreakIntent(forward: false, collapseSelection: true),
const SingleActivator(LogicalKeyboardKey.arrowRight, meta: true): const ExtendSelectionToLineBreakIntent(forward: true, collapseSelection: true),
......@@ -560,8 +560,8 @@ Intent? intentForMacOSSelector(String selectorName) {
'moveWordLeftAndModifySelection:': ExtendSelectionToNextWordBoundaryOrCaretLocationIntent(forward: false),
'moveWordRightAndModifySelection:': ExtendSelectionToNextWordBoundaryOrCaretLocationIntent(forward: true),
'moveParagraphBackwardAndModifySelection:': ExtendSelectionToLineBreakIntent(forward: false, collapseSelection: false, collapseAtReversal: true),
'moveParagraphForwardAndModifySelection:': ExtendSelectionToLineBreakIntent(forward: true, collapseSelection: false, collapseAtReversal: true),
'moveParagraphBackwardAndModifySelection:': ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent(forward: false),
'moveParagraphForwardAndModifySelection:': ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent(forward: true),
'moveToLeftEndOfLine:': ExtendSelectionToLineBreakIntent(forward: false, collapseSelection: true),
'moveToRightEndOfLine:': ExtendSelectionToLineBreakIntent(forward: true, collapseSelection: true),
......
......@@ -3978,6 +3978,7 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
TextBoundary _characterBoundary() => widget.obscureText ? _CodeUnitBoundary(_value.text) : CharacterBoundary(_value.text);
TextBoundary _nextWordBoundary() => widget.obscureText ? _documentBoundary() : renderEditable.wordBoundaries.moveByWordBoundary;
TextBoundary _linebreak() => widget.obscureText ? _documentBoundary() : LineBoundary(renderEditable);
TextBoundary _paragraphBoundary() => ParagraphBoundary(_value.text);
TextBoundary _documentBoundary() => DocumentBoundary(_value.text);
Action<T> _makeOverridable<T extends Intent>(Action<T> defaultAction) {
......@@ -4218,6 +4219,7 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
ExtendSelectionToLineBreakIntent: _makeOverridable(_UpdateTextSelectionAction<ExtendSelectionToLineBreakIntent>(this, _linebreak, _moveToTextBoundary, ignoreNonCollapsedSelection: true)),
ExtendSelectionVerticallyToAdjacentLineIntent: _makeOverridable(_verticalSelectionUpdateAction),
ExtendSelectionVerticallyToAdjacentPageIntent: _makeOverridable(_verticalSelectionUpdateAction),
ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent: _makeOverridable(_UpdateTextSelectionAction<ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent>(this, _paragraphBoundary, _moveBeyondTextBoundary, ignoreNonCollapsedSelection: true)),
ExtendSelectionToDocumentBoundaryIntent: _makeOverridable(_UpdateTextSelectionAction<ExtendSelectionToDocumentBoundaryIntent>(this, _documentBoundary, _moveBeyondTextBoundary, ignoreNonCollapsedSelection: true)),
ExtendSelectionToNextWordBoundaryOrCaretLocationIntent: _makeOverridable(_UpdateTextSelectionAction<ExtendSelectionToNextWordBoundaryOrCaretLocationIntent>(this, _nextWordBoundary, _moveBeyondTextBoundary, ignoreNonCollapsedSelection: true)),
ScrollToDocumentBoundaryIntent: _makeOverridable(CallbackAction<ScrollToDocumentBoundaryIntent>(onInvoke: _scrollToDocumentBoundary)),
......
......@@ -221,6 +221,21 @@ class ExtendSelectionVerticallyToAdjacentPageIntent extends DirectionalCaretMove
}) : super(forward, collapseSelection);
}
/// Extends, or moves the current selection from the current
/// [TextSelection.extent] position to the previous or the next paragraph
/// boundary depending on the [forward] parameter.
///
/// This [Intent] collapses the selection when the order of [TextSelection.base]
/// and [TextSelection.extent] would reverse.
///
/// This is typically only used on MacOS.
class ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent extends DirectionalCaretMovementIntent {
/// Creates an [ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent].
const ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent({
required bool forward,
}) : super(forward, false, true);
}
/// Extends, or moves the current selection from the current
/// [TextSelection.extent] position to the start or the end of the document.
///
......
......@@ -146,6 +146,113 @@ void main() {
expect(boundary.getTextBoundaryAt(3), TestTextLayoutMetrics.lineAt3);
});
group('paragraph boundary', () {
test('works for simple cases', () {
const String textA= 'abcd efg hi\njklmno\npqrstuv';
const ParagraphBoundary boundaryA = ParagraphBoundary(textA);
// Position enclosed inside of paragraph, 'abcd efg h|i\n'.
const int position = 10;
// The range includes the line terminator.
expect(boundaryA.getLeadingTextBoundaryAt(position), 0);
expect(boundaryA.getTrailingTextBoundaryAt(position), 12);
// This text includes a carriage return followed by a line feed.
const String textB = 'abcd efg hi\r\njklmno\npqrstuv';
const ParagraphBoundary boundaryB = ParagraphBoundary(textB);
expect(boundaryB.getLeadingTextBoundaryAt(position), 0);
expect(boundaryB.getTrailingTextBoundaryAt(position), 13);
const String textF = 'Now is the time for\n' // 20
'all good people\n' // 20 + 16 => 36
'to come to the aid\n' // 36 + 19 => 55
'of their country.'; // 55 + 17 => 72
const ParagraphBoundary boundaryF = ParagraphBoundary(textF);
const int positionF = 11;
expect(boundaryF.getLeadingTextBoundaryAt(positionF), 0);
expect(boundaryF.getTrailingTextBoundaryAt(positionF), 20);
});
test('works for consecutive line terminators involving CRLF', () {
const String textI = 'Now is the time for\n' // 20
'all good people\n\r\n' // 20 + 16 => 38
'to come to the aid\n' // 38 + 19 => 57
'of their country.'; // 57 + 17 => 74
const ParagraphBoundary boundaryI = ParagraphBoundary(textI);
const int positionI = 56;// \n at the end of the third line.
const int positionJ = 38;// t at beginning of third line.
const int positionK = 37;// \n at end of second line.
expect(boundaryI.getLeadingTextBoundaryAt(positionI), 38);
expect(boundaryI.getTrailingTextBoundaryAt(positionI), 57);
expect(boundaryI.getLeadingTextBoundaryAt(positionJ), 38);
expect(boundaryI.getTrailingTextBoundaryAt(positionJ), 57);
expect(boundaryI.getLeadingTextBoundaryAt(positionK), 36);
expect(boundaryI.getTrailingTextBoundaryAt(positionK), 38);
});
test('works for consecutive line terminators', () {
const String textI = 'Now is the time for\n' // 20
'all good people\n\n' // 20 + 16 => 37
'to come to the aid\n' // 37 + 19 => 56
'of their country.'; // 56 + 17 => 73
const ParagraphBoundary boundaryI = ParagraphBoundary(textI);
const int positionI = 55;// \n at the end of the third line.
const int positionJ = 37;// t at beginning of third line.
const int positionK = 36;// \n at end of second line.
expect(boundaryI.getLeadingTextBoundaryAt(positionI), 37);
expect(boundaryI.getTrailingTextBoundaryAt(positionI), 56);
expect(boundaryI.getLeadingTextBoundaryAt(positionJ), 37);
expect(boundaryI.getTrailingTextBoundaryAt(positionJ), 56);
expect(boundaryI.getLeadingTextBoundaryAt(positionK), 36);
expect(boundaryI.getTrailingTextBoundaryAt(positionK), 37);
});
test('leading boundary works for consecutive CRLF', () {
// This text includes multiple consecutive carriage returns followed by line feeds (CRLF).
const String textH = 'abcd efg hi\r\n\r\n\r\n\r\n\r\n\r\n\r\n\n\n\n\n\njklmno\npqrstuv';
const ParagraphBoundary boundaryH = ParagraphBoundary(textH);
const int positionH = 18;
expect(boundaryH.getLeadingTextBoundaryAt(positionH), 17);
expect(boundaryH.getTrailingTextBoundaryAt(positionH), 19);
});
test('trailing boundary works for consecutive CRLF', () {
// This text includes multiple consecutive carriage returns followed by line feeds (CRLF).
const String textG = 'abcd efg hi\r\n\n\n\n\n\n\r\n\r\n\r\n\r\n\n\n\n\n\njklmno\npqrstuv';
const ParagraphBoundary boundaryG = ParagraphBoundary(textG);
const int positionG = 18;
expect(boundaryG.getLeadingTextBoundaryAt(positionG), 18);
expect(boundaryG.getTrailingTextBoundaryAt(positionG), 20);
});
test('works when position is between two CRLF', () {
const String textE = 'abcd efg hi\r\nhello\r\n\n';
const ParagraphBoundary boundaryE = ParagraphBoundary(textE);
// Position enclosed inside of paragraph, 'abcd efg hi\r\nhello\r\n\n'.
const int positionE = 16;
expect(boundaryE.getLeadingTextBoundaryAt(positionE), 13);
expect(boundaryE.getTrailingTextBoundaryAt(positionE), 20);
});
test('works for multiple consecutive line terminators', () {
// This text includes multiple consecutive line terminators.
const String textC = 'abcd efg hi\r\n\n\n\n\n\n\n\n\n\n\n\njklmno\npqrstuv';
const ParagraphBoundary boundaryC = ParagraphBoundary(textC);
// Position enclosed inside of paragraph, 'abcd efg hi\r\n\n\n\n\n\n|\n\n\n\n\n\njklmno\npqrstuv'.
const int positionC = 18;
expect(boundaryC.getLeadingTextBoundaryAt(positionC), 18);
expect(boundaryC.getTrailingTextBoundaryAt(positionC), 19);
const String textD = 'abcd efg hi\r\n\n\n\n';
const ParagraphBoundary boundaryD = ParagraphBoundary(textD);
// Position enclosed inside of paragraph, 'abcd efg hi\r\n\n|\n\n'.
const int positionD = 14;
expect(boundaryD.getLeadingTextBoundaryAt(positionD), 14);
expect(boundaryD.getTrailingTextBoundaryAt(positionD), 15);
});
});
test('document boundary works', () {
const String text = 'abcd efg hi\njklmno\npqrstuv';
const DocumentBoundary boundary = DocumentBoundary(text);
......
......@@ -6365,6 +6365,161 @@ void main() {
);
expect(controller.text, equals(testText), reason: 'on $platform');
final bool platformCanSelectByParagraph = defaultTargetPlatform == TargetPlatform.iOS || defaultTargetPlatform == TargetPlatform.macOS;
// Move down one paragraph.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowDown,
],
shift: true,
wordModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(
selection,
equals(
TextSelection(
baseOffset: 10,
extentOffset: platformCanSelectByParagraph ? 20 : 10,
),
),
reason: 'on $platform',
);
// Move down another paragraph.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowDown,
],
shift: true,
wordModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(
selection,
equals(
TextSelection(
baseOffset: 10,
extentOffset: platformCanSelectByParagraph ? 36 : 10,
),
),
reason: 'on $platform',
);
// Move down another paragraph.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowDown,
],
shift: true,
wordModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(
selection,
equals(
TextSelection(
baseOffset: 10,
extentOffset: platformCanSelectByParagraph ? 55 : 10,
),
),
reason: 'on $platform',
);
// Move up a paragraph.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowUp,
],
shift: true,
wordModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(
selection,
equals(
TextSelection(
baseOffset: 10,
extentOffset: platformCanSelectByParagraph ? 36 : 10,
),
),
reason: 'on $platform',
);
// Move up a paragraph.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowUp,
],
shift: true,
wordModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(
selection,
equals(
TextSelection(
baseOffset: 10,
extentOffset: platformCanSelectByParagraph ? 20 : 10,
),
),
reason: 'on $platform',
);
// Move up back to the origin.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowUp,
],
shift: true,
wordModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(
selection,
equals(
TextSelection(
baseOffset: 10,
extentOffset: platformCanSelectByParagraph ? 10 : 10,
),
),
reason: 'on $platform',
);
// Move up, extending the selection backwards to the next paragraph.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowUp,
],
shift: true,
wordModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(
selection,
equals(
TextSelection(
baseOffset: 10,
extentOffset: platformCanSelectByParagraph ? 0 : 10,
),
),
reason: 'on $platform',
);
// Copy All
await sendKeys(
tester,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment