Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / lazarus / 2011-12

[patch] Debugger: CPU registers: resetting display formats on every restart of the application

11 messages · 2 people · –

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Bernd

This one has annoyed me a lot during the last few days while debugging some assembly code: The display of the cpu registers always reverted to rdDefault (which is decimal for the registers I was interested in) every time I restarted my application, it was so annoying and time consuming setting them back to hex (each of them individually!) on every restart that I started using kdbg instead of Lazarus.

The attached small patch for gdbmidebugger.pp would solve this problem once and for all. It will prevent the re-initialization of the FFormats array if it is already initialized (if it already has the correct size).

Would this patch qualify for inclusion?

Bernd

Attachment: gdbmidebugger_dont_reset_displayformats.patch -------------- next part --------------
A non-text attachment was scrubbed...
Name: gdbmidebugger_dont_reset_displayformats.patch Type: text/x-diff
Size: 1190 bytes
Desc: not available
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20111218/7f32645e/attachment.bin>

Reply by email

Martin
to Bernd
Bernd wrote 14 lines
This one has annoyed me a lot during the last few days while debugging some assembly code: The display of the cpu registers always reverted to rdDefault (which is decimal for the registers I was interested in) every time I restarted my application, it was so annoying and time consuming setting them back to hex (each of them individually!) on every restart that I started using kdbg instead of Lazarus. The attached small patch for gdbmidebugger.pp would solve this problem once and for all. It will prevent the re-initialization of the FFormats array if it is already initialized (if it already has the correct size). Would this patch qualify for inclusion?

Well thanks at first for your work.
The patch is almost ok.

The patch should check that the new list has the same registers in the same order.
I don't know all the condition under which the list can change at all, maybe if a 32bit exe is loaded on a 64 bit arch? And I don't know if ever there is a case where the count remains but not the names.

2 possibilities:
1) when you check the count, check the names as well
2) temporary keep the old names and formats in local vars; set the new list; search each new name in the old list and copy the format if found

The location of your patch is acceptable. i.e. the work currently needs be done in the procedure, that you picked.

Ideally, but that does not matter for your patch, the location should move. the whole current register display stuff is not ideal (I just went for a quick solution back then)

If ever the formats should be stored in the ide config (or project settings), then they must move. More info on request

Reply by email

Bernd
to Martin

2011/12/19 Martin :

The patch should check that the new list has the same registers in the same order.

The attached patch now stores all names of registers whose format has been changed (in SetFormat) and when initializing them again it looks up the preferred format by the register name. This is very similar to what I initially tried when I tried to do it from within the GUI.

Bernd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gdbmidebugger_dont_reset_displayformats_V2.patch Type: text/x-diff
Size: 2938 bytes
Desc: not available
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20111219/21ca7f3b/attachment.bin>

Reply by email

Bernd
to Bernd

and to make it complete here is another trivial patch to enable a popup menu in the registersdlg to be able to also set the display format with the right mouse button, this makes it more user friendly imho.

Bernd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: debugger_registersdlg_right_click_menu.patch Type: text/x-diff
Size: 1393 bytes
Desc: not available
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20111219/829a4deb/attachment.bin>

Reply by email

Martin
to Bernd
Bernd wrote 5 lines
and to make it complete here is another trivial patch to enable a popup menu in the registersdlg to be able to also set the display format with the right mouse button, this makes it more user friendly imho.

Why not simply assign the popup to the listview?

Reply by email

Martin
to Bernd: 2011/12/19 Martin : The attached patch now stores all names of registers whose format has…
Bernd wrote 10 lines
2011/12/19 Martin: > The patch should check that the new list has the same registers in the same > order. The attached patch now stores all names of registers whose format has been changed (in SetFormat) and when initializing them again it looks up the preferred format by the register name. This is very similar to what I initially tried when I tried to do it from within the GUI.

patch is ok.
which makes me feel guilty not to have used it as is.

But the mail you wrote made me see an alternative that I have implemented now (please test, r 34294)

Still thanks for it, your idea of the name/format record brought it to life.

First I have moved the formats by name into a class of it's own. Then I put an instance on the IDE side and allowed the gdb side to see a copy.

So now I can manage it on the IDE site and the gdb site has access

Storing by name works always. Because to access anything by index the Count is needed, and the count only exists if the names are known. So when accessing a format by index, there always is a name.

Reply by email

Bernd
to Martin

2011/12/19 Martin :

> Why not simply assign the popup to the listview?

When assigning it to the PopUpMenu poperty then it will not select the item before opening the popup (at least it will not move the visible selection until the popup is gone again). It seems this is meant for a menu that conceptually refers to the list as a whole but not the Item the user has clicked (at least on GTK2 it behaves like this, might this be a bug? IIRC I have read something regarding this problem that it cannot be solved differently on GTK2 which means the PopUpMenu property is pretty useless for a TListView). IMHO the user would normally always expect to *see* the selection bar moving to the clicked item before popping up the menu.

The only way I have found to have a popup menu fo a TListView that gives the user the proper visual feedback is using the OnMouseUp event..

Bernd

Reply by email

Martin
to Bernd
Bernd wrote 8 lines
2011/12/19 Martin: >> Why not simply assign the popup to the listview? When assigning it to the PopUpMenu poperty then it will not select the item before opening the popup (at least it will not move the visible selection until the popup is gone again). It seems this is meant for a menu that conceptually refers to the list as a whole but not the Item the user has clicked (at least on GTK2 it behaves like this, might this be a bug?

Ah, I will have to change it then

It worked on windows.

but the event order differs

Reply by email

Bernd
to Martin: patch is ok. which makes me feel guilty not to have used it as is. But the mail you wrote…

2011/12/19 Martin :

which makes me feel guilty not to have used it as is. But the mail you wrote made me see an alternative that I have implemented now (please test, r 34294)

No problem. It works, thanks for implementig it.

I almost expected you to rewrite it, I have also looked into putting this somewhere higher into the inheritance and make it more general but then decided to keep the impact small and not move too much stuff around since I know you have a much better overview over the workings of the debugger and probably already have precise ideas in mind how the architecture of all this should ideally look like.

Bernd

Reply by email

Martin
to Bernd
Bernd wrote 15 lines
2011/12/19 Martin: > which makes me feel guilty not to have used it as is. > > But the mail you wrote made me see an alternative that I have implemented > now (please test, r 34294) No problem. It works, thanks for implementig it. I almost expected you to rewrite it, I have also looked into putting this somewhere higher into the inheritance and make it more general but then decided to keep the impact small and not move too much stuff around since I know you have a much better overview over the workings of the debugger and probably already have precise ideas in mind how the architecture of all this should ideally look like.

Well after your first patch I was ready to go with a solution like yours, because I thought putting it higher up would take more time. (except the array had to be a list class, but that is trivial). Only when I saw your 2nd patch I realized there was an easy way.

In any case respect to how far you dug into the code. I remember my first encounters...

Eventually registers need a rewrite to use the new Monitor/Provider structure that watches use now. Then they can have history/snapshots...

But with having the format in a list-class now, it will be able to add load/save to xml (as watches have) and save them with the project.

Though I do not plan that for now, I can help you if you are interested ( I will be away the next 8 days though)

Reply by email