[Differential] [Requested Changes To] D2925: Port kdev_format_source script to C++/Qt
kfunk (Kevin Funk)
noreply at phabricator.kde.org
Mon Oct 17 06:50:57 UTC 2016
kfunk requested changes to this revision.
kfunk added a comment.
This revision now requires changes to proceed.
In https://phabricator.kde.org/D2925#56777, @antonanikin wrote:
> > 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.
>
> @kfunk, we should execute only one command per run - it's the original script behavior. It has simple sense - if we found some match we should apply only this command because we have "specialized" wildcards for some custom cases (which placed at the beginning of format file) and (usually) one "global" command ("*" wildcard) for all other cases.
I think it looks okay now, `KDevFormatFile::apply` makes it more explicit: there is a 1:1 relationship between format lines and command execution. For each format line, there's one call to `QProcess::execute` eventually => ok.
I'd still prefer if we could add a unit test here so we can be sure we don't break behavior now and in future.
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/20161017/b296678f/attachment.html>
More information about the KDevelop-devel
mailing list