Add History to Memory Viewer #1086
Conversation
Enhanced user experience by providing an history on the memory viewer with the last 50 explored addresses (can move backward or forward)
| @@ -287,6 +287,7 @@ void MemoryInspectorViewModel::OnCurrentAddressChanged(ra::ByteAddress nNewAddre | |||
| SetValue(CurrentAddressValueProperty, nValue); | |||
|
|
|||
| m_pViewer.SetAddress(nNewAddress); | |||
| m_pViewer.SaveToMemViewHistory(); | |||
There was a problem hiding this comment.
I was originally going to suggest you just capture the new address in SetAddress, but putting it here addresses another concern I had about the address being captured when navigating via the keyboard. Recommend having unit tests that ensure the history is not modified by keyboard navigation.
There was a problem hiding this comment.
I moved the AddToHistory directly inside SetAddress() as suggested.
Concerning capturing the keyboard navigation, I was thinking about that at some point and I found it way more convenient to capture it, it's too confusing if we don't track these movements :
Let's say we are on 0x0 then we move to 0x123 after which we start to use the keyboard to find something interresting somewhere near 0x123, after some digging we want to come back to 0x123 so we MoveBackward() but we reach 0x0.
If we want to get back to 0X123 we need to MoveForward() now, which is confusing.
There was a problem hiding this comment.
You don't want to capture keyboard movements. If you jump to address $1234, then press right and down to get to $1245, then type "ABCDEF", you'll be at $1248 and your history will be: [1234->1235->1245->1246->1247->1248]. If I went back in this scenario, I'd expect to jump back to $1234 (the last thing I jumped to), not to the last character I typed.
Testing in Visual Studio, it's a little more complicated. It doesn't remember where you jump to, it remembers where you jump from. If you click on row 6, character 8, then row 12, character 10, and press down and type "banana", when you hit back it goes to row 6, character 8. Then forward takes you to the position right after "banana".
- Update naming - Move the history list to vector - Add ClearHistory function - Refactor of the AddToHistory logic - Add unit tests
|
Committed ! Thanks @Jamiras ! |
|
This has conflicts from the +/- PR. |
Enhanced user experience by providing an history on the memory viewer with the last 50 explored addresses (can move backward or forward)