D17432: WIP: Add Analyzer Tool "Cppcheck (C++ only)"

Dominik Haumann noreply at phabricator.kde.org
Sun Dec 9 07:48:23 GMT 2018


dhaumann added a comment.


  I dislike the fact that we have two times almost the same thing. Then again I can see that if C++ is set explicitly, the cppcheck plugin will be unusable for C. Then again, cppcheck contains 'cpp' in its name. So should we care about C at all here? If the answer is 'no', I would prefer to simply add the command line option and be done with it :-) or add an option in the Projects config page.
  
  PS: i wrote this yesterday but forgot to click submit :p

INLINE COMMENTS

> kateprojectcodeanalysistoolcppcheck2.h:33-41
> +    virtual ~KateProjectCodeAnalysisToolCppcheck2() override;
> +
> +    virtual QString name() override;
> +
> +    virtual QString description() override;
> +
> +    virtual QString fileExtensions() override;

Unrelated to this patch:

- All getters should be const in the base class, right?
- virtual is not needed, since override already implies virtual.

> katemainwindow.h:373
> -     */
> -    bool closeSplitView(KTextEditor::View *view)
> -    {

If I am not mistaken: This should not be removed, since it's possibly called via the KTextEditor interface: https://github.com/KDE/ktexteditor/blob/master/src/utils/mainwindow.cpp#L152

Note this is a public Q_SLOT, meaning we can invoke this function via a QString trick we use to avoid binary compatibility issues... (not nice, but works!).

REPOSITORY
  R40 Kate

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

To: gregormi, #kate, cullmann
Cc: dhaumann, sars, cullmann, kwrite-devel, hase, michaelh, ngraham, demsking
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20181209/c01d26f9/attachment.html>


More information about the KWrite-Devel mailing list