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

Code before:
// parameter modulo the _height of the buffer, etc. But keep in mind that a offset=0
// is the GetScratchpadRow() and not the GetRowByOffset(0). That one is offset=1.
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);
}

// 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);
}

// Returns a row filled with whitespace and the current attributes, for you to freely use.
ROW& TextBuffer::GetScratchpadRow()
{
    return GetScratchpadRow(_currentAttributes);
}

// Returns a row filled with whitespace and the given attributes, for you to freely use.
ROW& TextBuffer::GetScratchpadRow(const TextAttribute& attributes)
{

Review comment (suggestion):
I realized that the
```cpp
const auto y = (_commitWatermark - _buffer.get() - 1) / _bufferRowStride;
```
calculation is effectively the same as
```cpp
const auto x = (_commitWatermark - _buffer.get()) / _bufferRowStride;
const auto y = x - 1;
```
and so it kind of merges with the other -1 we already had to form -2.
I also felt like it's neat because now it's only 2 lines of code and the `std::max` ensures it's impossible to return negative indices.

Code after:
// parameter modulo the _height of the buffer, etc. But keep in mind that a offset=0
// is the GetScratchpadRow() and not the GetRowByOffset(0). That one is offset=1.
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;
    }