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

gregormi noreply at phabricator.kde.org
Sun Dec 9 09:22:49 GMT 2018


gregormi added a comment.


  In D17432#373749 <https://phabricator.kde.org/D17432#373749>, @dhaumann wrote:
  
  > 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.
  
  
  I like the idea:
  
  1. Restrict the original "Cppcheck (C++)" to C++ and remove the ".c" file extension from the list (and still do not search header files since these seem to generate false positives)
  2. Add another version "Cppcheck (C)" which is set to "c" and then only search within .c files.

INLINE COMMENTS

> dhaumann wrote in kateprojectcodeanalysistoolcppcheck2.h:33-41
> Unrelated to this patch:
> 
> - All getters should be const in the base class, right?
> - virtual is not needed, since override already implies virtual.

about const: yes, after rebase these will be const

> dhaumann wrote in katemainwindow.h:373
> 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!).

Ah, good to know. I found this occurrence:

frameworks/ktexteditor/src/utils/mainwindow.cpp
146:bool MainWindow::closeSplitView(KTextEditor::View *view)
153:                              , "closeSplitView"

Christoph also mentioned it. I will revert this change.

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/028bbd10/attachment.html>


More information about the KWrite-Devel mailing list