Fix the hyperlink detection when there are leading wide glyph in the row

[comzyh]

Summary of the Pull Request

URL detection was broken again in #15858. When the regex matched, we calculate the column(cell) by its offset, we use forward or backward iteration of the column to find the correct column that displays the glyphs of _chars[offset].

terminal/src/buffer/out/Row.cpp

Lines 95 to 104 in abf5d94

	til::CoordType CharToColumnMapper::GetLeadingColumnAt(ptrdiff_t offset) noexcept
	{
	offset = clamp(offset, 0, _lastCharOffset);
	auto col = _currentColumn;
	const auto currentOffset = _charOffsets[col];
	// Goal: Move the _currentColumn cursor to a cell which contains the given target offset.
	// Depending on where the target offset is we have to either search forward or backward.
	if (offset < currentOffset)

However, when calculating the currentOffset we forget that MSB of _charOffsets[col] could be 1, or col is pointing to another glyph in preceding column.

terminal/src/buffer/out/Row.hpp

Lines 223 to 226 in abf5d94

	// for _charOffsets. Instead we use the most significant bit to indicate whether any column is the
	// trailing half of a wide glyph. This simplifies many implementation details via _uncheckedIsTrailer.
	static constexpr uint16_t CharOffsetsTrailer = 0x8000;
	static constexpr uint16_t CharOffsetsMask = 0x7fff;

[comzyh]

Before:

After:

@lhecker
also the test case for reproduce

安装最新的 PowerShell，了解新功能和改进！https://aka.ms/PSWindows

[lhecker]

First of all, I deeply apologize for breaking this.

I'm astonished you figured out how to fix this! This is rather deep in our code base after all. If you ever have any questions, please feel free to ask us. 🙂 I've edited your PR a bit to remove the images, since the commit may be archived for a long time and GitHub may not be around anymore at that point.

[comzyh]

Thanks.
I just feel the following part still has some bugs. When looking backward, it seems we will return 1 more than we expected.

terminal/src/buffer/out/Row.cpp

Lines 115 to 117 in de0f702

	for (; offset < _charOffsets[col - 1]; --col)
	{
	}

you can try:

𝓐𝓐𝓐!https://aka.ms/PSWindows

[lhecker]

I've merged your PR as I'm planning to add some explanatory comments on top of your changes. Thanks for letting me know about the other issue you've found. I'll try to fix that with my next PR. I'm hoping to get to that later today or tomorrow. 🙂

[lhecker]

I've figured it out. It's really that I forgot to use the CharOffsetsMask but rather that this loop should not subtract 1 from col:

terminal/src/buffer/out/Row.cpp

Lines 115 to 117 in bb5f56e

	for (; offset < _charOffsets[col - 1]; --col)
	{
	}

In other words, this is correct:

if (offset < _charOffsets[col])
{
    for (; offset < _charOffsets[col]; --col)
    {
    }
}
// ...

It immediately makes way more sense this way, since the if condition and for loop now match perfectly.

But looking at it again, I now realize that the entire code is way too complex. I've simplified it massively to just this:

til::CoordType CharToColumnMapper::GetLeadingColumnAt(ptrdiff_t targetOffset) noexcept
{
    targetOffset = clamp(targetOffset, 0, _lastCharOffset);

    auto col = _currentColumn;
    auto currentOffset = _charOffsets[col];

    while (targetOffset > (currentOffset & CharOffsetsMask))
    {
        currentOffset = _charOffsets[++col];
    }
    while (targetOffset < currentOffset)
    {
        currentOffset = _charOffsets[--col];
    }

    _currentColumn = col;
    return col;
}

That's it. Just 2 loops. Makes it way more robust and faster too. I'll submit a PR soon.

Thank you so much again for finding these bugs!