https://bugs.kde.org/show_bug.cgi?id=362485
Igor Kushnir <igor...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
Latest Commit| |https://invent.kde.org/kdev
| |elop/kdevelop/-/commit/a152
| |4591a3d0d02a493c3742d2c772e
| |9f7b50654
Version Fixed In| |5.14
--- Comment #6 from Igor Kushnir <igor...@gmail.com> ---
Git commit a1524591a3d0d02a493c3742d2c772e9f7b50654 by Igor Kushnir.
Committed on 08/03/2024 at 10:22.
Pushed by igorkushnir into branch 'master'.
BreakpointModel: complete document reload support
This commit addresses the following issue and removes the FIXME comment
that describes it: if a modified document is reloaded, and the user
clicks a button other than "Discard" in the message box that appears,
the moving cursors should not be reverted to their saved locations.
When a modified document is reloaded, the user makes a
Cancel/Discard/Save choice after the aboutToReload() signal is emitted.
Therefore, BreakpointModel::aboutToReload() must not make any changes to
breakpoints or breakpoint marks that depend on this user choice. Remove
moving cursors during a reload later - in
BreakpointModel::aboutToInvalidateMovingInterfaceContent() instead of
BreakpointModel::aboutToReload().
If the user cancels reloading a modified document, it does not emit the
signal aboutToInvalidateMovingInterfaceContent(), its moving cursors are
not invalidated and breakpoints remain intact.
If the user reloads a modified document and opts to save it (and
overwrite possible changes on disk), BreakpointModel::documentSaved() is
invoked and saves the breakpoints' tracked line numbers before
aboutToInvalidateMovingInterfaceContent() removes the moving cursors.
If the user reloads a modified document and opts to discard editor
changes or reloads an unmodified document, the executed BreakpointModel
code is almost unchanged, because these cases were already perfectly
supported. The only consequential difference is that aboutToReload()
inhibits mark change instead of removing all breakpoint marks in the
reloaded document. That's because the breakpoint marks must not be
removed in case the user cancels reloading.
Unfortunately, not removing breakpoint marks in aboutToReload() lets
KTextEditor::DocumentPrivate::documentReload() store in a local variable
all of the document's breakpoint marks. The saving and restoring of
breakpoint marks by KTextEditor is useless work, because BreakpointModel
has a different notion/algorithm where breakpoint marks should be placed
after the document is reloaded. There is no API to prevent this useless
KTextEditor work. So BreakpointModel resorts to working around possible
mark conflicts by removing all breakpoint marks in the document before
adding them at correct locations in case of the Discard user choice. In
case of the Save user choice or when an unmodified document is reloaded,
KTextEditor restores breakpoint marks at correct locations.
BreakpointModel still restores breakpoint marks again afterwards,
because KTextEditor may choose to skip restoring some of the marks. This
double restoration is safe, because KTextEditor::Document's member
function addMark(int line, uint markType) does not change anything if a
mark of the type `markType` is already set on the line `line`.
Connect to 3 KTextEditor::Document's signals - aboutToReload(),
aboutToInvalidateMovingInterfaceContent() and reloaded() in
BreakpointModel::textDocumentCreated(). The downside of connecting to
every open document's signals is that both the connecting itself and the
work in the slots when the document is reloaded is useless if there are
no breakpoints at its URL (which should be the case for most documents).
I have considered and rejected connecting to these 3 signals in
BreakpointModel::setupMovingCursor(), which is called only for documents
that contain breakpoints. The problem is that a moving cursor is not set
up for a breakpoint at a line out of the document's range. But reloading
may increase the number of lines in the document and thus allow
BreakpointModel::reloaded() to enable document line tracking for
newly-in-range breakpoints.
In the future, BreakpointModel should acquire a way to efficiently
access all breakpoints in a given document. This would substantially
reduce overhead for documents without breakpoints and allow to move the
existing aboutToDeleteMovingInterfaceContent() connection from
setupMovingCursor() to textDocumentCreated().
A unit test cannot make the Cancel/Discard/Save user choice
programmatically instead of showing the Close Document warning dialog.
Add semiautomatic tests and skip them by default. When a developer makes
code changes that affect breakpoints in a reloaded modified document,
[s]he can disable the QSKIP() call and run the tests.
Extract two helper functions from
TestBreakpointModel::testDocumentReload() and reuse them. The extracted
code is practically unchanged.
FIXED-IN: 5.14
M +6 -0 kdevplatform/debugger/breakpoint/breakpoint.cpp
M +7 -0 kdevplatform/debugger/breakpoint/breakpoint.h
M +107 -31 kdevplatform/debugger/breakpoint/breakpointmodel.cpp
M +4 -3 kdevplatform/debugger/breakpoint/breakpointmodel.h
M +170 -27 kdevplatform/debugger/tests/test_breakpointmodel.cpp
M +8 -0 kdevplatform/debugger/tests/test_breakpointmodel.h
https://invent.kde.org/kdevelop/kdevelop/-/commit/a1524591a3d0d02a493c3742d2c772e9f7b50654
--
You are receiving this mail because:
You are watching all bug changes.
