Repository: microsoft/terminal
File: src/buffer/out/textBuffer.cpp (C++)

Code before:
ROW& TextBuffer::_getRowByOffsetDirect(size_t offset)
{
    const auto row = _buffer.get() + _bufferRowStride * offset;
    THROW_HR_IF(E_UNEXPECTED, row < _buffer.get() || row >= _bufferEnd);

    if (row >= _commitWatermark)
    {
        _commit(row);
    }

    return *reinterpret_cast<ROW*>(row);
}

// This function is "direct" because it trusts the caller to know that this row
// could be the scratchpad row.
size_t TextBuffer::_estimateOffsetOfLastCommittedRowDirect() const noexcept
{
    if (_commitWatermark == _buffer.get())
    {
        // Estimation failed. Act as though we have at least one non-scratchpad row committed.
        // The caller will be passing this offset back to GetRowByOffset or similar later, which
        // will commit it.
        return 1;
    }
    const auto lastRowOffset = ((_commitWatermark - 1) - _buffer.get()) / _bufferRowStride;
    return gsl::narrow_cast<size_t>(lastRowOffset);
}

// Returns the "user-visible" index of the last committed row, which can be used
// to short-circuit some algorithms that try to scan the entire buffer..
size_t TextBuffer::_estimateOffsetOfLastCommittedRow() const noexcept
{
    // Take into account the scratchpad row.
    return _estimateOffsetOfLastCommittedRowDirect() - 1;
}

// Retrieves a row from the buffer by its offset from the first row of the text buffer
// (what corresponds to the top row of the screen buffer).
const ROW& TextBuffer::GetRowByOffset(const til::CoordType index) const
{
    // The const_cast is safe because "const" never had any meaning in C++ in the first place.
#pragma warning(suppress : 26492) // Don't use const_cast to cast away const or volatile (type.3).
    return const_cast<TextBuffer*>(this)->GetRowByOffset(index);
}

// Retrieves a row from the buffer by its offset from the first row of the text buffer
// (what corresponds to the top row of the screen buffer).
ROW& TextBuffer::GetRowByOffset(const til::CoordType index)
{
    // Rows are stored circularly, so the index you ask for is offset by the start position and mod the total of rows.
    auto offset = (_firstRow + index) % _height;

Review comment (refactor):
nit: Purely theoretically speaking, if `_commitReadAheadRowCount` was 0, this could return 0 instead of 1. It's not really an issue at all, but this code only works because other code works in particular way. (I mean I also write tons of code like that, but... it's objectively not quite perfect IMO.)

From my personal perspective I also wouldn't consider the scratchpad row a "real" row and so I personally don't think `_estimateOffsetOfLastCommittedRowDirect` is worth keeping. `_getRowByOffsetDirect` only exists because I wanted to deduplicate the two row accessors, but I've also considered comitting the first viewport worth of rows eagerly so that we can just blindly assume that `_buffer.get()` is always committed and thus return that pointer as the scratchpad row instantly.

In short, I'd inline `_estimateOffsetOfLastCommittedRowDirect` into `_estimateOffsetOfLastCommittedRow` and return 0 above.

Code after:
ROW& TextBuffer::_getRowByOffsetDirect(size_t offset)
{
    const auto row = _buffer.get() + _bufferRowStride * offset;
    THROW_HR_IF(E_UNEXPECTED, row < _buffer.get() || row >= _bufferEnd);

    if (row >= _commitWatermark)
    {
        _commit(row);
    }

    return *reinterpret_cast<ROW*>(row);
}

// Returns the "user-visible" index of the last committed row, which can be used
// to short-circuit some algorithms that try to scan the entire buffer.
// Returns 0 if no rows are committed in.
til::CoordType TextBuffer::_estimateOffsetOfLastCommittedRow() const noexcept
{
    const auto lastRowOffset = (_commitWatermark - _buffer.get()) / _bufferRowStride;
    // This subtracts 2 from the offset to account for the:
    // * scratchpad row at offset 0, whereas regular rows start at offset 1.
    // * fact that _commitWatermark points _past_ the last committed row,
    //   but we want to return an index pointing at the last row.
    return std::max(0, gsl::narrow_cast<til::CoordType>(lastRowOffset - 2));
}

// Retrieves a row from the buffer by its offset from the first row of the text buffer
// (what corresponds to the top row of the screen buffer).
const ROW& TextBuffer::GetRowByOffset(const til::CoordType index) const
{
    // The const_cast is safe because "const" never had any meaning in C++ in the first place.
#pragma warning(suppress : 26492) // Don't use const_cast to cast away const or volatile (type.3).
    return const_cast<TextBuffer*>(this)->GetRowByOffset(index);
}

// Retrieves a row from the buffer by its offset from the first row of the text buffer
// (what corresponds to the top row of the screen buffer).
ROW& TextBuffer::GetRowByOffset(const til::CoordType index)
{
    // Rows are stored circularly, so the index you ask for is offset by the start position and mod the total of rows.
    auto offset = (_firstRow + index) % _height;

    // Support negative wrap around. This way an index of -1 will
    // wrap to _rowCount-1 and make implementing scrolling easier.
    if (offset < 0)
    {
        offset += _height;
    }

    // We add 1 to the row offset, because row "0" is the one returned by GetScratchpadRow().
    return _getRowByOffsetDirect(gsl::narrow_cast<size_t>(offset) + 1);