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

[Milian Wolff]

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?
  
  Thanks

INLINE COMMENTS

> 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?

REPOSITORY
  R33 KDevPlatform

REVISION DETAIL
  https://phabricator.kde.org/D4334

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

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>