Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

Splitting debugger in two - specific debugger and common GUI

<< < (82/136) > >>

cbexaminr:
dbgextprocess.2(b).patch (b after recent trunk to debugger branch merges)

1)Change various text macro uses, and empty string "" to wxEmptyString
2)Verify that GUI is not gdb specific, but is used for both gdb and cdb (and try Debug external
process with cdb - see item 4)
3)Add cleanup of some overlooked vars on end of Debug external process
4)Fix bug introduced in recent cdb changes affecting Attach to process (new code in GetCommonCommandLine() referencing NULL m_Target) that also affected Debug external process

Awaiting advice on what to do differently than IsExternalProcess()...

oBFusCATed:

--- Quote from: cbexaminr on February 08, 2011, 02:13:25 pm ---2)Verify that GUI is not gdb specific, but is used for both gdb and cdb (and try Debug external
process with cdb - see item 4)

--- End quote ---
Here I'm not talking about CDB, but the new python debugger or another kind of debugger, that can be used only in remote debugging mode (the lua debugger will be just like that if someday I start implementing it). My idea is to move the calling code inside the debugger, so you have the option to implement it or not.
This way the interface is a bit smaller... Also the GUI has too many thing in it.


--- Quote from: cbexaminr on February 08, 2011, 02:13:25 pm ---Awaiting advice on what to do differently than IsExternalProcess()...

--- End quote ---
This one requires major refactoring. If it doesn't bother you, use your version for now. I'll give it a try myself next week, I think I'll have a bit of free time.

cbexaminr:

--- Quote from: oBFusCATed on February 08, 2011, 03:19:06 pm ---... If it doesn't bother you, use your version for now...

--- End quote ---

The current state of the code on which I've already implemented the patch (and brought it forward several times in the past few months)
...does not bother me as much as...
having a patch with functionality 
A)that I find useful,
B)that is (or has been in the past) common functionality in at least 3 other IDEs (borland/cg/embt, visual studio, codelite) so is likely useful to others as well,
C)that does not seem to be philosophically out-of-line with other functionality in the patch target, and
D)that can currently be applied to the code as it is,
E)that does not seem to have any definite practical reason for not being applied

...with a delay in applying it until after some not-yet-begun architectural changes

a)requiring further work on someone's part (to bring it forward yet again) that could be avoided by its application, and
b)the lack of that functionality (no access to it) for others to whom it might be useful now or in the unknown interval when intended architectural changes and required (patch) rework are completed.
c)requires further work on my part to maintain and stay current with the debugger branch for whatever benefit that might provide me (and those working on it for whom I might be able to provide feedback in areas I use)

But, if you do not wish to apply, and the admins agree with that choice, then you may want to at least fix the crash it also addresses in cdb_driver::GetCommonCommandLine() when Attach to process is used with no project opened (which appears to have been introduced with the last previous patches for cdb).

MortenMacFly:

--- Quote from: cbexaminr on February 08, 2011, 08:11:55 pm ---...does not bother me as much as...

--- End quote ---
Please guys, calm down. I believe oBFusCATed is truly willing to accept changes / patches that address the functionalities you have in mind. It is the same for me but I guess all that's missing for now is some more time. I for myself would be willing to try but simply don't have enough time, but I can understand your frustration coming out of this situation.


--- Quote from: cbexaminr on February 08, 2011, 08:11:55 pm ---the admins agree with that choice

--- End quote ---
I cannot agree atm nor I disagree. When I look at the patch I see a lot TBD's so it seems unfinished to me. We already have a lot TBD's in the debugger plugin so adding more doesn't seem wise from an "outside" point of you. You see: Usually patches we apply are "finished"... even in the development branch (have a look at the history). The goal is to get the branch stable so it can be integrated into trunk. So I hope you also understand our point of view which is no way is disrespecting your work, btw! Every help is absolutely welcome!

However, as a compromise how about applying it for a nightly to give a chance to other users to try. If the community benefits from it we can polish out the ToDo's. But this should clearly be done in a continuous work.

dmoore:

--- Quote from: oBFusCATed on February 05, 2011, 06:49:56 pm ---Another options is

--- Code: ---MyPlugins::OnAttach(...)
{
    cbDebugger::OnAttach(...);
    ....
}

--- End code ---

--- End quote ---

I actually slightly prefer this because most plugin devs are familiar with OnAttach and OnRelease, and calling a base class method is a familiar idiom (esp. in the wxWidgets world). I don't think OnAttachReal, DoAttach and friends are all that helpful to newbies, because they are basically synonyms of OnAttach that don't convey the nuanced meaning. Anyway, this is a style thing so I'm not going to push too hard (because I'm notorious for bad style).

I have plenty questions about the Debugging API. As a general comment, the cbDebuggerPlugin class definition could definitely use some more documentation, as I'm not always sure what the framework is providing and what the plugin needs to do (lack of time on your part, I know). A couple of specific questions (there will be more):

1. which is responsible for setting and moving the code position marker in the editor: the plugin or the framework? Assuming the framework, which methods need to be implemented in the plugin (and what other calls need to be made or events signalled) to get this to happen?
2. I noticed the the GDB_MI plugin was using some sort of smart pointer to store the breakpoint class instances (with the breakpoint class itself defined in the SDK). I would have thought the framework would provide the container classes...

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version