The doc on TextPainter says "The newline, if any, is included in the range". However this doesn't seem to be the case... or I missunderstand this statement.

I couldn't find any tests for TextPainter.getLineBoundary to see if this is expected.

Oh, interesting. So the web tests failed here with:

07:30 +485 ~12 -1: /b/s/w/ir/k/flutter/packages/flutter/test/rendering/paragraph_test.dart: getLineBoundary control test [E]                                                                           
  Expected: 'I polished up that handle so carefullee'
    Actual: 'I polished up that handle so carefullee\n'
              ''
     Which: is different. Both strings start the same, but the actual value also has the following trailing characters: \n

Which sounds like implementation for the web platform is more aligned with the doc... ?

Running this test locally on macOS works fine.

I'm probably going to need help determining which behavior is the correct one and whether I should be looking at the TextPainter implementation to fix this inconsistency.

I would agree with you, something seems not right there. Let's see if @GaryQian knows what the behavior should be.

There are some tests in the web part of the engine for this, and they include the newline: https://github.com/flutter/engine/blob/09a5bf7acf44dbb22d68830f5a51ebe822d5e659/lib/web_ui/test/paragraph_test.dart#L926

This may indeed be a bug, I think the newline makes sense here, especially since the docs indicate that it was meant to be included. It isn't your responsibility to fix that though, so you may want to just mark it with a comment and a TODO: blahblah to indicate that the result here may be due to a bug. Then, you can duplicate the test and make one of them skip web while having the other skip android+ios. We should file a bug on this.

Extra newline here.

required => require

Or maybe this? "Accessing this value does not cause the layout to be updated."

Removed this method per @GaryQian 's recommendation.

Any of these docs comments that were copied from text_painter.dart should use a macro instead.

Here's an example of how to do that:

Thanks for the examples!

I would agree with you, something seems not right there. Let's see if @GaryQian knows what the behavior should be.

There are some tests in the web part of the engine for this, and they include the newline: https://github.com/flutter/engine/blob/09a5bf7acf44dbb22d68830f5a51ebe822d5e659/lib/web_ui/test/paragraph_test.dart#L926

If you have a workaround for preferredLineHeight, I would prefer that we hold off on exposing this as official API. The current implementation of this value is sketchy at best and is something I want to eventually rework.

Some problems with this value is that it assumes an alphabetic space as the character to get metrics from. This causes some issues with ideographic text such as Chinese or even Arabic in some fonts. The current implementation also does not account for the differences in first line vs second+ line height, so could produce values that are not useful beyond the first line.

For now, I suggest continuing to use your workaround instead of adding this API that may have to be broken in the future.

Makes sense, thanks! I can keep my workaround for now and remove this method from here.

tight -> strut

Can you change this in the original docs in TextPainter as well? This doc is no longer accurate.

Going off of the macro comment by Justin, change this in the original doc, and then macro it here.

Updated and added a macro in TextPainter. I haven't included the last line referencing [layout] of TextPainter as it seemed specific to the painter class itself (even though RenderParagraph has a similar requirement).

Let me know if you'd like that line to be included in the template, I'll update.

Also changed [position] to use backticks, as it seems more inline with the style guide around method parameters.

The primary reason this is not exposed is API bloat, since once we add it, it cannot be removed. We should try our best to explore reasonable alternatives before resorting to adding additional API.

In this case, getLineBoundary exposes a few of the text position APIs based on metrics, which we want to eventually move away from to contain more bitdepth.

I think you may be able to workaround this by calling getBoxesForSelection, once with a collapsed selection to get the rect that bounds the current position, and again with a wide selection that encompasses a few lines. Then, you can take the right most and left most boundaries of the boxes that share the same y offset as the first call. Then pass those boundaries to getPositionForOffset.

That should get you something that may work for your purposes. If the resulting code is really complicated, confusing, or unreliable, we can proceed with exposing this.

Let me know if you are able to get a workaround working!

Yep it does seem to work pretty well. Thanks for the suggestion! Since each RenderParagraph in Zefyr represents a single paragraph of text (meaning text between two newlines) here is what I ended up with:

https://github.com/memspace/zefyr/blob/78a5ac11542d39a45cc11cb904416dec0c443da7/packages/zefyr/lib/src/rendering/editable_text_line.dart#L294-L309

  @override
  TextRange getLineBoundary(TextPosition position) {
    // getOffsetForCaret returns top-left corner of the caret. To find all
    // selection boxes on the same line we shift caret offset by 0.5 of
    // preferredLineHeight so that it's in the middle of the line and filter out
    // boxes which do not include this offset on the Y axis.
    final caret = getOffsetForCaret(position);
    final lineDy = caret.translate(0.0, 0.5 * preferredLineHeight(position)).dy;
    final boxes = getBoxesForSelection(
        TextSelection(baseOffset: 0, extentOffset: node.length - 1));
    final lineBoxes = boxes
        .where((element) => element.top < lineDy && element.bottom > lineDy)
        .toList(growable: false);
    final start = getPositionForOffset(Offset(lineBoxes.first.left, lineDy));
    final end = getPositionForOffset(Offset(lineBoxes.last.right, lineDy));
    return TextRange(start: start.offset, end: end.offset);
  }

I'm checking at the vertical middle of the line because I've noticed that emojis, for instance, have larger selection boxes (and they also mess with the line height they are at):

Overall it's actually pretty concise and I'm happy with this approach. So I removed getLineBoundary from this PR.

It now only exposes getFullHeightForCaret.

This should be ok to expose, as the CaretMetrics construct is native to TextPainter.

The name "fullHeight" seems a bit ambiguous in hindsight especially since there is no API for not-full height here. We may want to consider renaming it to something like "rawHeight" here, indicating it is unprocessed and a direct result of the caret computation. The alternative is to just keep the naming as "full" for consistency.

I am leaning towards keeping it as "full" as diverging API is not good. @justinmc any opinions?

Is there anything else that refers to "full height" in the codebase? Not that I could think of or find with a quick search.

If it's really just this method, maybe we should nip it in the bud and switch to "rawHeight" while we have the chance? If I'm wrong then it definitely makes sense to be consistent and use "fullHeight".

Per @Hixie suggestion I updated this test to be more strict in comparing expected value.

Oh yeah, one more thing before submitting, caretPrototype here does not do anything useful as in getFullHeightForCaret, it only contributes width, which is then ignored. You should just remove the parameter and pass a zero rect instead in the method body.

Updated.