Auto-fill the instruction text when adding a TTD bookmark (Fixes #1108) - #1151
Auto-fill the instruction text when adding a TTD bookmark (Fixes #1108)#1151xusheng6 wants to merge 1 commit into
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fuzyll
left a comment
fuzyll
left a comment
There was a problem hiding this comment.
Genuinely not sure if I should approve or not. If you go through these and none of them are applicable, let me know and I'll approve. But, I do think there probably need to be some changes, even if they're small.
| m_noteEdit->setMinimumWidth(400); | ||
| layout->addRow("Note:", m_noteEdit); | ||
|
|
||
| // Pre-fill the note with the instruction at the view address, so the bookmark is meaningful without the user |
There was a problem hiding this comment.
Is the logic here correct? I haven't built this and ran it, but if m_noteEdited is !note.isEmpty (above, line 140), doesn't that mean that deleting a note and leaving it empty will cause it to auto-fill?
If this is intentional/desired, awesome, I just didn't understand that from the comment here. Seemed like an edge-case that wasn't addressed instead.
| @@ -216,7 +216,7 @@ void TTDCallsQueryWidget::setupUIActions() | |||
| defaultNote = funcItem->text(); | |||
There was a problem hiding this comment.
Re: Earlier comment about auto-fill, it looks like this will prevent that. Again, if that's desired, awesome. Just stuck out as a difference.
| QString posStr = posItem->text(); | ||
|
|
||
| TTDBookmarkEditDialog dialog(this, posStr, "", ""); | ||
| TTDBookmarkEditDialog dialog(this, posStr, "", "", m_controller); |
There was a problem hiding this comment.
I also ran an LLM review on this PR as I had some questions on how certain things worked in the debugger after finishing my review. When I did, it said there is a problem here. I asked it for more information, and it said:
Exception bookmarks omit the available program counter. The dialog receives an empty view address even though exception rows contain a PC, so these bookmarks neither autofill the instruction nor retain the relevant navigation address.
The bookmark dialog can only generate instruction text when it receives a View Address:
uint64_t address = ParseHexAddress(m_viewAddressEdit->text());
if (!data || (address == 0))
return "";
In TTDEventsQueryWidget, exception rows already display the exception’s program counter. For example,ui/ttdeventswidget.cpp:731populates the Program Counter column fromevent.exception->programCounter.
However, the bookmark action always constructs the dialog with an empty view address:
TTDBookmarkEditDialog dialog(this, posStr, "", "", m_controller);
That third empty string means:
- The dialog cannot disassemble the instruction at the exception PC.
- The generated note remains empty.
- The saved bookmark’s viewAddress remains zero.
- Navigating to that bookmark later changes the TTD position but cannot return the UI to the exception instruction.
For example, an exception at position123:4with PC0x140001234creates a bookmark for123:4, but discards0x140001234.
Looking over this, I think its logic holds?
2 participants
Prefill the disassembly text of the current instruction as notes when adding a TTD bookmark