[Differential] [Requested Changes To] D2925: Port kdev_format_source script to C++/Qt

[kfunk (Kevin Funk)]

kfunk requested changes to this revision.
kfunk added a reviewer: kfunk.
kfunk added a comment.
This revision now requires changes to proceed.


  Please add a unit test which calls `kdev_format_source` on a few (simple) format files.

INLINE COMMENTS

> kdev_format_source.cpp:52
> +
> +        QFile formatFile(formatFileName);
> +        if (!formatFile.open(QIODevice::ReadOnly | QIODevice::Text)) {

Factor out the detection of the format file, e.g. `findFormatFile(...)`. This also reduces the indent of the following lines.

> kdev_format_source.cpp:58
> +
> +        int line_number = 0;
> +        bool matched = false;

Style: CamelCase

> kdev_format_source.cpp:64
> +
> +        while (!formatFile.atEnd()) {
> +            ++line_number;

I'd try to separate the steps a) parsing the format file + b) executing the commands in the format file.

For parsing, something simple as `QVector<FormatLine> parseFormatFile()`, where `FormatLine` is a `struct { QVector<QString> wildcards; QString command; }`. Then use this function.

> kdev_format_source.cpp:108
> +
> +        if (matched && !command.isEmpty()) {
> +            if (wildcard.isEmpty())

Something's fishy here. This is not in the correct scope is it? If I understand correctly, then there can only be one call to `QProcess::execute(command)` per run? This can't be correct as there may be multiple format lines in a format file; each of them executing a different command. Please check.

REPOSITORY
  rKDEVPLATFORM KDevPlatform

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: antonanikin, #kdevelop, kfunk
Cc: kfunk, apol, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20161012/8c30ca6e/attachment-0001.html>