[Differential] [Requested Changes To] D4334: Pressing "Continue" starts "Debug" if program is not running

Milian Wolff noreply at phabricator.kde.org
Sun Jan 29 19:48:45 UTC 2017

mwolff requested changes to this revision.
mwolff added a reviewer: mwolff.
mwolff added a comment.
This revision now requires changes to proceed.

  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?


> debugcontroller.cpp:477
>      }
> +    else{
> +        if(ICore::self()->runController()->launchConfigurations().isEmpty()) {

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

> debugcontroller.cpp:478
> +    else{
> +        if(ICore::self()->runController()->launchConfigurations().isEmpty()) {
> +            LaunchConfigurationDialog d;

space after if

> debugcontroller.cpp:482
> +        }
> +        ICore::self()->runController()->executeDefaultLaunch(QStringLiteral("debug"));
> +    }

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?

> kdevdebuggershellui.rc:36
>      <State name="ended">
> -        <disable>
> +        <enable>
>              <Action name="debug_continue"/>

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?

  R33 KDevPlatform



To: dporobic, apol, kfunk, mwolff
Cc: mwolff, kdevelop-devel, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170129/75d3476b/attachment.html>

More information about the KDevelop-devel mailing list