D14288: Initial version of Clazy analyzer plugin

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> commandlinewidget.ui:29
> +     <property name="title">
> +      <string>Command line</string>
> +     </property>

Since there are still changes to do... this needs to be "Command Line", since it is the title of a group box.
See our guidelines on capitalization: https://hig.kde.org/style/writing/capitalization.html

> globalconfigpage.cpp:38
> +        ChecksDB db(ui.kcfg_docsPath->url());
> +        ui.checksInfoLabel->setText(i18n("%1 checks detected", db.checks().size()));
> +

This must be i18np(), since there is a variable number.

> globalconfigpage.ui:52
> +        <property name="text">
> +         <string>clazy docs:</string>
> +        </property>

Please no abbreviations -- see https://hig.kde.org/style/writing/wording.html

> globalconfigpage.ui:59
> +        <property name="toolTip">
> +         <string>Path to clazy docs directory</string>
> +        </property>

Ditto

> projectconfigpage.ui:56
> +            <property name="toolTip">
> +             <string>Won't emit warnings for non-Qt files, or in other words, if -DQT_CORE_LIB is missing</string>
> +            </property>

No contractions please

> projectconfigpage.ui:169-170
> +            <property name="text">
> +             <string>WARNING: Backup your code and don't blame me if a fixit is not applied correctly.
> +For better results don't use parallel checks, otherwise a fixit being applied in an header file might be done twice.</string>
> +            </property>

- no contractions
- no need to SCREAM
- "do not blame me": other than being unfriendly, users do not even know who to blame...

> projectconfigpage.ui:187
> +      <attribute name="title">
> +       <string>Extra parameters</string>
> +      </attribute>

"Extra Parameters"

> job.cpp:97
> +
> +    scriptStream << QStringLiteral("SOURCES = %1\n").arg(clazySources.join(' '));
> +

Will this work if the sources are in paths with spaces (eg `/data/my directory/devel/`)?

> job.cpp:100
> +    if (GlobalSettings::verboseOutput()) {
> +        scriptStream << QStringLiteral("COMMAND = %2\n").arg(clazyCommand);
> +    } else {

Ditto, even the clazy command can be somewhere with a path with eg spaces.

> jobparameters.cpp:155
> +    if (!QFile::exists(commandsFilePath)) {
> +        m_error = i18n("%1 does not exists", commandsFilePath);
> +        return;

"exist"

> antonanikin wrote in test_clazyjob.cpp:82
> Clazy is runtime dependency, so I think we should always run the test.

Exactly, it is a runtime dependency: if I build kdevelop and run the test suite with no clazy installed, this test fails. As Friedrich said, a failure means something went wrong, not that an optional tool was not found.

> utils.cpp:91-93
> +// Very simple Markdown parser/converter. Does not provide full Markdown language support and
> +// was tested only with Clazy documentation.
> +class MarkdownConverter

FWIW: Okular uses the Discount library <http://www.pell.portland.or.us/~orc/Code/discount/> for reading markdown documents, showing them as HTML.  Also Cantor will do so too: D14738: Add the markdown entry <https://phabricator.kde.org/D14738>.

Maybe it'd be a good idea to use it -- there are few options:

- make the library a hard dependency for this plugin (which then would not be built if discount is not found
- optionally use discount, using this simple fallback otherwise (though with the risk of rotting)
- optionally use discount, and show no documentation if not found

REPOSITORY
  R32 KDevelop

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

To: antonanikin, #kdevelop
Cc: mwolff, apol, kfunk, brauch, pino, kossebau, kdevelop-devel, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20180819/9b987637/attachment-0001.html>