D11234: [DrKonqi] Show debug button when KDevelop session is running

[Harald Sitter]

sitter requested changes to this revision.
sitter added a comment.
This revision now requires changes to proceed.


  addDebugger seems a bit much, no? Why don't you just have a m_debugButtonInBox member to track whether it is currently added or not? When it is already in the buttonBox running the for loop over the buttons is entirely unnecessary.

INLINE COMMENTS

> drkonqidialog.cpp:190
> +    // Only add the debugger if requested by the config or if a KDevelop session is running.
> +    const bool showexternal = debuggerManager->showExternalDebuggers();
>      QList<AbstractDebuggerLauncher*> debuggers = debuggerManager->availableExternalDebuggers();

CamelCase

> drkonqidialog.cpp:193
>      foreach(AbstractDebuggerLauncher *launcher, debuggers) {
> -        addDebugger(launcher);
> +        if (showexternal || qobject_cast<DBusInterfaceLauncher*>(launcher))
> +            addDebugger(launcher);

We always use curly braces for ifs in Plasma

> drkonqidialog.cpp:248
> +    // inserted, then add the other buttons. See removeDebugger below for more information.
> +    bool hasdebugbutton{false};
> +    QVector<QAbstractButton*> buttons;

CamelCase

Should be `= false`

> drkonqidialog.cpp:250
> +    QVector<QAbstractButton*> buttons;
> +    for (QAbstractButton *b : m_buttonBox->buttons()) {
> +        if (b == m_debugButton) {

No single letter variables please.

> drkonqidialog.cpp:261
> +        m_debugButton->setVisible(true);
> +        for (QAbstractButton *b : buttons) {
> +            m_buttonBox->addButton(b, QDialogButtonBox::ActionRole);

No single letter variables please.

REPOSITORY
  R871 DrKonqi

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

To: croick, #plasma_workspaces, apol, mwolff, #kdevelop, sitter
Cc: plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190611/4d987048/attachment.html>