D17971: Revive External Tools plugin

[loh tar]

loh.tar added a comment.


  Here the link to the mentioned asyle stuff
  https://community.kde.org/Policies/Kdelibs_Coding_Style#Artistic_Style_.28astyle.29_automatic_code_formatting
  But there is still  "--align-pointer=name" missing

INLINE COMMENTS

> externaltoolsplugin.cpp:233
> +
> +    KateExternalToolsPluginView* pluginView = runner->view() ? viewForMainWindow(runner->view()->mainWindow()) : nullptr;
> +    if (pluginView) {

runner->view() is your view

> externaltoolsplugin.cpp:262
> +
> +    delete runner;
> +}

Looks to me that in case of auto view == nullptr you could return on top of function and avoid all further tests

> externaltoolsplugin.h:53
> +    /**
> +     * Reimplemented to instanciate a PluginView for each MainWindow.
> +     */

instantiate ?

> externaltoolsplugin.h:70
> +    /**
> +     * Returns the KateExternalTool for a specific command line command 'cmd.
> +     */

\@p cmd ?

> kateexternaltool.h:65
> +    QString name;
> +    /// the icon to use in the menu.
> +    QString icon;

T

> kateexternaltool.h:80
> +    QString actionName;
> +    /// The name for the commandline.
> +    QString cmdname;

command line ?

> kateexternaltool.h:86
> +    bool reload = false;
> +    /// Defines where to redirect the tool's output
> +    OutputMode outputMode = OutputMode::Ignore;

you had always dots at the end, same one line above

> kateexternaltool.h:90
> +public:
> +    /// This is set when loading the Tool from disk.
> +    bool hasexec = false;

Is is set when load the tool from disk. ?

> kateexternaltoolsconfigwidget.cpp:76
> +
> +// BEGIN KateExternalToolServiceEditor
> +KateExternalToolServiceEditor::KateExternalToolServiceEditor(KateExternalTool* tool, QWidget* parent)

Existing code use //BEGIN (no space)

> kateexternaltoolsconfigwidget.cpp:257
> +    if (!m_changed)
> +        return;
> +    m_changed = false;

{ }

> kateexternaltoolsconfigwidget.cpp:330
> +{
> +    // searach for existing category
> +    auto items = m_toolsModel.findItems(category);

search

> kateexternaltoolsconfigwidget.h:41
> + * The config widget.
> + * The config widget allows the user to view a list of services of the type
> + * "Kate/ExternalTool" and add, remove or edit them.

rm  The config widget, starting with Allows

> kateexternaltoolsconfigwidget.h:42
> + * The config widget allows the user to view a list of services of the type
> + * "Kate/ExternalTool" and add, remove or edit them.
> + */

and to

> kateexternaltoolsconfigwidget.h:116
> +    /**
> +     * show a mimetype chooser dialog
> +     */

S

> kateexternaltoolsconfigwidget.h:138
> +     */
> +    ContextAction(const QIcon & icon, const QString & text, QObject * parent = nullptr);
> +

Coding style says QIcon &icon, always to the right. You reformat by some clang command? There was a astyle command somewhere offered. There are much more other places affected.

> kateexternaltoolsview.h:91
> +    /**
> +     * Returns the associated mainWindow
> +     */

dot, also next comment

> kateexternaltoolsview.h:123
> +    /**
> +     * Sets the output data to data;
> +     */

; -> .

some cases above toolview -> tool view

> katetoolrunner.h:74
> +    /**
> +     * Blocking call that waits until the tool is finised.
> +     * Used internally for unit testing.

Blockingcall ... finished. or done?

REPOSITORY
  R40 Kate

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

To: dhaumann, cullmann, gregormi
Cc: loh.tar, brauch, ngraham, kwrite-devel, gennad, domson, michaelh, demsking, cullmann, sars, dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20190315/caa519a2/attachment-0001.html>