Repository: microsoft/terminal
File: src/cascadia/TerminalControl/TermControl.cpp (C++)

Code before:
    const size_t TermControl::TaskbarProgress() const noexcept
    {
        return _core->TaskbarProgress();
    }

    void TermControl::BellLightOn()
    {
        if (!_bellLightTimer)
        {
            // Start the timer, when the timer ticks we switch off the light
            DispatcherTimer invertTimer;
            invertTimer.Interval(std::chrono::milliseconds(TerminalWarningBellInterval));
            invertTimer.Tick({ get_weak(), &TermControl::_BellLightOff });
            invertTimer.Start();
            _bellLightTimer.emplace(std::move(invertTimer));

            // Switch on the light and animate the intensity to fade out
            VisualBellLight::SetIsTarget(RootGrid(), true);
            BellLight().CompositionLight().StartAnimation(L"Intensity", _bellLightAnimation);
        }
    }

    void TermControl::_BellLightOff(Windows::Foundation::IInspectable const& /* sender */,
                                    Windows::Foundation::IInspectable const& /* e */)
    {
        if (_bellLightTimer && !_closing)
        {
            // Stop the timer and switch off the light
            _bellLightTimer.value().Stop();
            _bellLightTimer = std::nullopt;
            VisualBellLight::SetIsTarget(RootGrid(), false);
        }
    }

    // Method Description:
    // - Checks whether the control is in a read-only mode (in this mode node input is sent to connection).
    // Return Value:
    // - True if the mode is read-only
    bool TermControl::ReadOnly() const noexcept
    {
        return _core->IsInReadOnlyMode();
    }

    // Method Description:
    // - Toggles the read-only flag, raises event describing the value change
    void TermControl::ToggleReadOnly()
    {
        _core->ToggleReadOnlyMode();
        _ReadOnlyChangedHandlers(*this, winrt::box_value(_core->IsInReadOnlyMode()));
    }


Review comment (suggestion):
I believe you should theoretically also stop the timer if the application is closing. You can also use the `->` operator instead of `.value()` and `.reset()` for clearing the optional.
For instance:

```cpp
if (!_bellLightTimer)
{
	return;
}

_bellLightTimer->Stop();
_bellLightTimer.reset();

if (_closing)
{
	return;
}

VisualBellLight::SetIsTarget(RootGrid(), false);
```

(I personally like "linear" flows like this, but it would be just as fine if you write it with nested if conditions.)

Code after:
    {
        return _core->TaskbarProgress();
    }

    void TermControl::BellLightOn()
    {
        Windows::Foundation::Numerics::float2 zeroSize{ 0, 0 };
        // If the grid has 0 size or if the bell timer is
        // already active, do nothing
        if (RootGrid().ActualSize() != zeroSize && !_bellLightTimer)
        {
            // Start the timer, when the timer ticks we switch off the light
            DispatcherTimer invertTimer;
            invertTimer.Interval(std::chrono::milliseconds(TerminalWarningBellInterval));
            invertTimer.Tick({ get_weak(), &TermControl::_BellLightOff });
            invertTimer.Start();
            _bellLightTimer.emplace(std::move(invertTimer));

            // Switch on the light and animate the intensity to fade out
            VisualBellLight::SetIsTarget(RootGrid(), true);
            BellLight().CompositionLight().StartAnimation(L"Intensity", _bellLightAnimation);
        }
    }

    void TermControl::_BellLightOff(Windows::Foundation::IInspectable const& /* sender */,
                                    Windows::Foundation::IInspectable const& /* e */)
    {
        if (_bellLightTimer)
        {
            // Stop the timer and switch off the light
            _bellLightTimer->Stop();
            _bellLightTimer.reset();

            if (!_closing)
            {
                VisualBellLight::SetIsTarget(RootGrid(), false);
            }
        }
    }

    // Method Description:
    // - Checks whether the control is in a read-only mode (in this mode node input is sent to connection).
    // Return Value:
    // - True if the mode is read-only
    bool TermControl::ReadOnly() const noexcept
    {
        return _core->IsInReadOnlyMode();
    }

    // Method Description:
    // - Toggles the read-only flag, raises event describing the value change