[web] Initial support for LineMetrics#13985
Conversation
| final int endWithoutSpace = _excludeTrailing( | ||
| _text, | ||
| _lineStart, | ||
| lineEnd, |
There was a problem hiding this comment.
I think, since newlines are a subset of whitespace, we could pass endWithoutNewlines here instead of lineEnd, and avoid scanning from the end twice.
| @required int start, | ||
| @required int end, | ||
| }) { | ||
| assert(0 <= start && start < end && end <= _text.length); |
There was a problem hiding this comment.
nit: when this assert fails it will say that 0 <= start && start < end && end <= _text.length evaluated to false. I think breaking it up into separate asserts - assert(0 <= start); assert(start < end); assert(end <= _text.length) - would provide a more actionable error message.
There was a problem hiding this comment.
Done. Also applied the same change to few other asserts in this file.
|
|
||
| @override | ||
| String toString() { | ||
| return 'EngineLineMetrics(lineNumber: $lineNumber, text: "$text", hardBreak: $hardBreak, width: $width)'; |
There was a problem hiding this comment.
This string could show up in user code (there was a recent test breakage when some web class went out of sync). Let's make sure we're consistent with the C++ version.
There was a problem hiding this comment.
This is only useful for tests now, so I'll hide it behind assertionsEnabled. The C++ version doesn't implement it. It just return the default Instance of 'LineMetrics'.
There was a problem hiding this comment.
Maybe add the same implementation in the C++ version?
There was a problem hiding this comment.
Removed the method.
mdebbar
force-pushed
the
line_metrics
branch
from
fa4d1fb to
09b752d
Compare
* 29b2e91 Include uncommitted files when checking for code format issues. (flutter/engine#14035) * 7a7118d Roll src/third_party/skia 861ac61cb1a9..c96f5108df28 (10 commits) (flutter/engine#14059) * bc07e6a [web] Initial support for LineMetrics (flutter/engine#13985) * bcb8267 Revert "Do not default to downstream affinity on iOS insertText (#13852)" (flutter/engine#14053) * 4bdd15c more tests for enabling semantics (flutter/engine#14060)
3 participants
Part of flutter/flutter#33523
Fixes flutter/flutter#33613
Fixes flutter/flutter#39537