Pressing "Continue" starts "Debug" if program is not running
ClosedPublic

Authored by dporobic on Jan 29 2017, 1:49 PM.

Details

Summary

As requested in bug 342245 (https://bugs.kde.org/show_bug.cgi?id=342245).

Changes the kdevdebuggershellui to make the "continue" action visible when the debugger is not running, otherwise the "continue" action would be disabled.
Triggering the "continue" action when while no debug is running will have the same effect as triggering "debug" action. The "continue" action for running debugs remains the same as before.

Test Plan

Manual Testing

Diff Detail

Repository
R33 KDevPlatform
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dporobic updated this revision to Diff 10674.Jan 29 2017, 1:49 PM
dporobic retitled this revision from to Pressing "Continue" starts "Debug" if program is not running.
dporobic updated this object.
dporobic edited the test plan for this revision. (Show Details)
dporobic set the repository for this revision to R33 KDevPlatform.
dporobic added a project: KDevelop.
dporobic added reviewers: apol, kfunk.
dporobic added a subscriber: kdevelop-devel.
mwolff requested changes to this revision.Jan 29 2017, 7:48 PM
mwolff added a reviewer: mwolff.
mwolff added a subscriber: mwolff.

I am missing some things here, besides the nitpicks below:

The action's tooltip, and possibly even its icon, should now imo update depending on the state to indicate what action will be triggered when it is executed.

Can you implement that as well please?

Thanks

shell/debugcontroller.cpp
477

join to next line, add space after else, such that it reads: } else {

478

space after if

482

if the dialog gets canceled, we may still not have a launch, right? will that still work fine (i.e. no crash/warning), or should that be catched explicitly here?

shell/debugger/kdevdebuggershellui.rc
36

couldn't you simply remove the debug_continue altogether from both states, since if you never disable it, you also never have to reenable it again?

This revision now requires changes to proceed.Jan 29 2017, 7:48 PM
dporobic updated this revision to Diff 10736.Jan 30 2017, 6:21 PM
dporobic edited edge metadata.
dporobic removed R33 KDevPlatform as the repository for this revision.

+ Added a function that changes the icon and text of the continue_debug action to look like launch_debug action depending on the debugger state.
+ Changed formatting as suggested by mwolff.

dporobic marked 2 inline comments as done.Jan 30 2017, 6:29 PM
In D4334#81123, @mwolff wrote:

I am missing some things here, besides the nitpicks below:

The action's tooltip, and possibly even its icon, should now imo update depending on the state to indicate what action will be triggered when it is executed.

Can you implement that as well please?

Thanks

I've implemented some functionality to fix this, can you please have a look if this is ok or if I should take a different approach.

Thanks!

shell/debugcontroller.cpp
482

A crash should not happen, further execution will be stopped here and a warning will be written to console. It's the same why how the real debug action is implemented.

void KDevelop::RunController::executeDefaultLaunch(const QString& runMode)
{
    auto dl = defaultLaunch();
    if( !dl )
    {
        qWarning() << "no default launch!";
        return;
    }
    execute( runMode, dl );
}
shell/debugger/kdevdebuggershellui.rc
36

I am setting it to disabled, during the "active" state and it looks more clear when all action are there. But I can remove them if you think that this is useless.

mwolff accepted this revision.Jan 30 2017, 11:00 PM
mwolff edited edge metadata.

one minor nitpick, otherwise lgtm

do you have commit rights? if so, feel free to amend and push to master

shell/debugcontroller.cpp
433

rename to: setContinueStartsDebug

This revision is now accepted and ready to land.Jan 30 2017, 11:00 PM
dporobic updated this revision to Diff 10775.Jan 31 2017, 3:57 PM
dporobic edited edge metadata.

Changed function name that changes the continue action properties from continueStartsDebug to setContinueStartsDebug

In D4334#81627, @mwolff wrote:

one minor nitpick, otherwise lgtm

do you have commit rights? if so, feel free to amend and push to master

Changed as suggested.

I do not have a developer account to commit my patch. Could you do that?

mwolff added a comment.Feb 1 2017, 6:04 PM

I can commit, but I need your email address and name so I can preserve that information in the git commit. Phabricator does not show me that information :-/

Sure, damir_porobic(at)live(dot)com would be the address and name is Damir Porobic.

This revision was automatically updated to reflect the committed changes.